From c8564ad47627c29878127bcb6edfa09808198462 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Fri, 8 Jun 2012 15:01:08 -0600
Subject: [PATCH] virsh: make tree listing more flexible

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 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 9194a5ba3e..4dd1d81e3a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13165,10 +13165,15 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
 }
 
 /* Tree listing helpers.  */
+
+/* Given an index, return either the name of that device (non-NULL) or
+ * of its parent (NULL if a root).  */
+typedef const char * (*vshTreeLookup)(int devid, bool parent, void *opaque);
+
 static int
 vshTreePrintInternal(vshControl *ctl,
-                     char **devices,
-                     char **parents,
+                     vshTreeLookup lookup,
+                     void *opaque,
                      int num_devices,
                      int devid,
                      int lastdev,
@@ -13178,13 +13183,14 @@ vshTreePrintInternal(vshControl *ctl,
     int i;
     int nextlastdev = -1;
     int ret = -1;
+    const char *dev = (lookup)(devid, false, opaque);
 
     if (virBufferError(indent))
         goto cleanup;
 
     /* Print this device, with indent if not at root */
     vshPrint(ctl, "%s%s%s\n", virBufferCurrentContent(indent),
-             root ? "" : "+- ", devices[devid]);
+             root ? "" : "+- ", dev);
 
     /* Update indent to show '|' or ' ' for child devices */
     if (!root) {
@@ -13196,10 +13202,10 @@ vshTreePrintInternal(vshControl *ctl,
 
     /* Determine the index of the last child device */
     for (i = 0 ; i < num_devices ; i++) {
-        if (parents[i] &&
-            STREQ(parents[i], devices[devid])) {
+        const char *parent = (lookup)(i, true, opaque);
+
+        if (parent && STREQ(parent, dev))
             nextlastdev = i;
-        }
     }
 
     /* If there is a child device, then print another blank line */
@@ -13209,8 +13215,10 @@ vshTreePrintInternal(vshControl *ctl,
     /* Finally print all children */
     virBufferAddLit(indent, "  ");
     for (i = 0 ; i < num_devices ; i++) {
-        if (parents[i] && STREQ(parents[i], devices[devid]) &&
-            vshTreePrintInternal(ctl, devices, parents,
+        const char *parent = (lookup)(i, true, opaque);
+
+        if (parent && STREQ(parent, dev) &&
+            vshTreePrintInternal(ctl, lookup, opaque,
                                  num_devices, i, nextlastdev,
                                  false, indent) < 0)
             goto cleanup;
@@ -13230,13 +13238,13 @@ cleanup:
 }
 
 static int
-vshTreePrint(vshControl *ctl, char **devices, char **parents,
+vshTreePrint(vshControl *ctl, vshTreeLookup lookup, void *opaque,
              int num_devices, int devid)
 {
     int ret;
     virBuffer indent = VIR_BUFFER_INITIALIZER;
 
-    ret = vshTreePrintInternal(ctl, devices, parents, num_devices,
+    ret = vshTreePrintInternal(ctl, lookup, opaque, num_devices,
                                devid, devid, true, &indent);
     if (ret < 0)
         vshError(ctl, "%s", _("Failed to complete tree listing"));
@@ -13244,6 +13252,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];
+}
+
 /*
  * "nodedev-list" command
  */
@@ -13293,6 +13315,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     qsort(&devices[0], num_devices, sizeof(char*), namesorter);
     if (tree) {
         char **parents = vshMalloc(ctl, sizeof(char *) * num_devices);
+        struct vshTreeArray arrays = { devices, parents };
 
         for (i = 0; i < num_devices; i++) {
             virNodeDevicePtr dev = virNodeDeviceLookupByName(ctl->conn, devices[i]);
@@ -13306,7 +13329,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
         }
         for (i = 0 ; i < num_devices ; i++) {
             if (parents[i] == NULL &&
-                vshTreePrint(ctl, devices, parents, num_devices, i) < 0)
+                vshTreePrint(ctl, vshTreeArrayLookup, &arrays, num_devices,
+                             i) < 0)
                 ret = false;
         }
         for (i = 0 ; i < num_devices ; i++) {
@@ -16755,10 +16779,12 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
         }
     }
     if (tree) {
+        struct vshTreeArray arrays = { names, parents };
+
         for (i = 0 ; i < actual ; i++) {
             if ((from && ctl->useSnapshotOld) ? STREQ(names[i], from) :
                 !parents[i] &&
-                vshTreePrint(ctl, names, parents, actual, i) < 0)
+                vshTreePrint(ctl, vshTreeArrayLookup, &arrays, actual, i) < 0)
                 goto cleanup;
         }