I am not a fan of fixed-width buffers. All it takes is a
linear chain of more than 100 snapshots to mess up 'virsh
snapshot-list --tree'. Now that virBuffer is more powerful,
we might as well exploit its power.
* tools/virsh.c (cmdNodeListDevicesPrint): Simplify to use a
virBuffer instead of fixed-width prefix, factor guts, and rename...
(vshTreePrint, vshTreePrintInternal): ...along with new helper.
(cmdNodeListDevices, cmdSnapshotList): Update callers.
---
tools/virsh.c | 172 ++++++++++++++++++++++++++-------------------------------
1 file changed, 78 insertions(+), 94 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index b28dc49..0d67f4b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13165,60 +13165,34 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
return true;
}
-/*
- * "nodedev-list" command
- */
-static const vshCmdInfo info_node_list_devices[] = {
- {"help", N_("enumerate devices on this host")},
- {"desc", ""},
- {NULL, NULL}
-};
-
-static const vshCmdOptDef opts_node_list_devices[] = {
- {"tree", VSH_OT_BOOL, 0, N_("list devices in a tree")},
- {"cap", VSH_OT_STRING, VSH_OFLAG_NONE, N_("capability name")},
- {NULL, 0, 0, NULL}
-};
-
-#define MAX_DEPTH 100
-#define INDENT_SIZE 4
-#define INDENT_BUFLEN ((MAX_DEPTH * INDENT_SIZE) + 1)
-
-static void
-cmdNodeListDevicesPrint(vshControl *ctl,
- char **devices,
- char **parents,
- int num_devices,
- int devid,
- int lastdev,
- unsigned int depth,
- unsigned int indentIdx,
- char *indentBuf)
+/* Tree listing helpers. */
+static int
+vshTreePrintInternal(vshControl *ctl,
+ char **devices,
+ char **parents,
+ int num_devices,
+ int devid,
+ int lastdev,
+ bool root,
+ virBufferPtr indent)
{
int i;
int nextlastdev = -1;
+ int ret = -1;
- /* Prepare indent for this device, but not if at root */
- if (depth && depth < MAX_DEPTH) {
- indentBuf[indentIdx] = '+';
- indentBuf[indentIdx+1] = '-';
- indentBuf[indentIdx+2] = ' ';
- indentBuf[indentIdx+3] = '\0';
- }
-
- /* Print this device */
- vshPrint(ctl, "%s", indentBuf);
- vshPrint(ctl, "%s\n", devices[devid]);
+ 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]);
/* Update indent to show '|' or ' ' for child devices */
- if (depth && depth < MAX_DEPTH) {
- if (devid == lastdev)
- indentBuf[indentIdx] = ' ';
- else
- indentBuf[indentIdx] = '|';
- indentBuf[indentIdx+1] = ' ';
- indentIdx+=2;
+ if (!root) {
+ virBufferAddChar(indent, devid == lastdev ? ' ' : '|');
+ virBufferAddChar(indent, ' ');
+ if (virBufferError(indent))
+ goto cleanup;
}
/* Determine the index of the last child device */
@@ -13230,43 +13204,70 @@ cmdNodeListDevicesPrint(vshControl *ctl,
}
/* If there is a child device, then print another blank line */
- if (nextlastdev != -1) {
- vshPrint(ctl, "%s", indentBuf);
- vshPrint(ctl, " |\n");
- }
+ if (nextlastdev != -1)
+ vshPrint(ctl, "%s |\n", virBufferCurrentContent(indent));
/* Finally print all children */
- if (depth < MAX_DEPTH)
- indentBuf[indentIdx] = ' ';
+ virBufferAddLit(indent, " ");
for (i = 0 ; i < num_devices ; i++) {
- if (depth < MAX_DEPTH) {
- indentBuf[indentIdx] = ' ';
- indentBuf[indentIdx+1] = ' ';
- }
- if (parents[i] &&
- STREQ(parents[i], devices[devid]))
- cmdNodeListDevicesPrint(ctl, devices, parents,
- num_devices, i, nextlastdev,
- depth + 1, indentIdx + 2, indentBuf);
- if (depth < MAX_DEPTH)
- indentBuf[indentIdx] = '\0';
+ if (parents[i] && STREQ(parents[i], devices[devid]) &&
+ vshTreePrintInternal(ctl, devices, parents,
+ num_devices, i, nextlastdev,
+ false, indent) < 0)
+ goto cleanup;
}
+ virBufferTrim(indent, " ", -1);
/* If there was no child device, and we're the last in
* a list of devices, then print another blank line */
- if (nextlastdev == -1 && devid == lastdev) {
- vshPrint(ctl, "%s", indentBuf);
- vshPrint(ctl, "\n");
- }
+ if (nextlastdev == -1 && devid == lastdev)
+ vshPrint(ctl, "%s\n", virBufferCurrentContent(indent));
+
+ if (!root)
+ virBufferTrim(indent, NULL, 2);
+ ret = 0;
+cleanup:
+ return ret;
+}
+
+static int
+vshTreePrint(vshControl *ctl, char **devices, char **parents,
+ int num_devices, int devid)
+{
+ int ret;
+ virBuffer indent = VIR_BUFFER_INITIALIZER;
+
+ ret = vshTreePrintInternal(ctl, devices, parents, num_devices,
+ devid, devid, true, &indent);
+ if (ret < 0)
+ vshError(ctl, "%s", _("Failed to complete tree listing"));
+ virBufferFreeAndReset(&indent);
+ return ret;
}
+/*
+ * "nodedev-list" command
+ */
+static const vshCmdInfo info_node_list_devices[] = {
+ {"help", N_("enumerate devices on this host")},
+ {"desc", ""},
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_node_list_devices[] = {
+ {"tree", VSH_OT_BOOL, 0, N_("list devices in a tree")},
+ {"cap", VSH_OT_STRING, VSH_OFLAG_NONE, N_("capability name")},
+ {NULL, 0, 0, NULL}
+};
+
static bool
-cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
{
const char *cap = NULL;
char **devices;
int num_devices, i;
bool tree = vshCommandOptBool(cmd, "tree");
+ bool ret = true;
if (!vshConnectionUsability(ctl, ctl->conn))
return false;
@@ -13292,8 +13293,8 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
}
qsort(&devices[0], num_devices, sizeof(char*), namesorter);
if (tree) {
- char indentBuf[INDENT_BUFLEN];
char **parents = vshMalloc(ctl, sizeof(char *) * num_devices);
+
for (i = 0; i < num_devices; i++) {
virNodeDevicePtr dev = virNodeDeviceLookupByName(ctl->conn, devices[i]);
if (dev && STRNEQ(devices[i], "computer")) {
@@ -13305,17 +13306,9 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
virNodeDeviceFree(dev);
}
for (i = 0 ; i < num_devices ; i++) {
- memset(indentBuf, '\0', sizeof(indentBuf));
- if (parents[i] == NULL)
- cmdNodeListDevicesPrint(ctl,
- devices,
- parents,
- num_devices,
- i,
- i,
- 0,
- 0,
- indentBuf);
+ if (parents[i] == NULL &&
+ vshTreePrint(ctl, devices, parents, num_devices, i) < 0)
+ ret = false;
}
for (i = 0 ; i < num_devices ; i++) {
VIR_FREE(devices[i]);
@@ -13329,7 +13322,7 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
}
}
VIR_FREE(devices);
- return true;
+ return ret;
}
/*
@@ -16763,20 +16756,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
}
}
if (tree) {
- char indentBuf[INDENT_BUFLEN];
for (i = 0 ; i < actual ; i++) {
- memset(indentBuf, '\0', sizeof(indentBuf));
if ((from && ctl->useSnapshotOld) ? STREQ(names[i], from) :
- !parents[i])
- cmdNodeListDevicesPrint(ctl,
- names,
- parents,
- actual,
- i,
- i,
- 0,
- 0,
- indentBuf);
+ !parents[i] &&
+ vshTreePrint(ctl, names, parents, actual, i) < 0)
+ goto cleanup;
}
ret = true;
--
1.7.10.2
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list