[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/6] virsh: make tree listing more flexible



On 06/09/12 06:34, Eric Blake wrote:
Requiring the user to pass in parallel arrays of names and parents
is annoying; it means that you can't qsort one of the arrays without
invalidating the ordering of the other.  By refactoring this function
to use callbacks, we isolate the layout to be independent of the
printing, and a future patch can exploit that to improve layout.

* tools/virsh.c (vshTreePrintInternal): Use callbacks rather than
requiring a char** array.
(vshTreeArrayLookup): New helper function.
(vshTreePrint, cmdNodeListDevices, cmdSnapshotList): Update callers.
---
  tools/virsh.c |   50 ++++++++++++++++++++++++++++++++++++++------------
@@ -13245,6 +13253,20 @@ vshTreePrint(vshControl *ctl, char **devices, char **parents,
      return ret;
  }

+struct vshTreeArray {
+    char **names;
+    char **parents;
+};
+
+static const char *
+vshTreeArrayLookup(int devid, bool parent, void *opaque)
+{
+    struct vshTreeArray *arrays = opaque;
+    if (parent)
+        return arrays->parents[devid];
+    return arrays->names[devid];
+}
+

This is used just for listing node devices IIUC. Wouln't it be better to explicitly name the helper and struct that it's meant for node devices? vshNodeDeviceTree ... ?

  /*
   * "nodedev-list" command
   */

ACK. The name doesn't matter that much, making the tree printer more universal does.

Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]