[libvirt] [PATCH] virsh: simply printing of typed parameters
No need to repeat code for formatting typed parameters. * tools/virsh.c (vshGetTypedParamValue): Support strings. (cmdSchedinfo, cmdBlkiotune, cmdMemtune, cmdBlkdeviotune): Use it for less code. --- tools/virsh.c | 134 +++++++++------------------------------------------------ 1 files changed, 21 insertions(+), 113 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3654589..a3ec7e9 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2975,28 +2975,9 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) ret_val = true; for (i = 0; i < nparams; i++) { - switch (params[i].type) { - case VIR_TYPED_PARAM_INT: - vshPrint(ctl, "%-15s: %d\n", params[i].field, params[i].value.i); - break; - case VIR_TYPED_PARAM_UINT: - vshPrint(ctl, "%-15s: %u\n", params[i].field, params[i].value.ui); - break; - case VIR_TYPED_PARAM_LLONG: - vshPrint(ctl, "%-15s: %lld\n", params[i].field, params[i].value.l); - break; - case VIR_TYPED_PARAM_ULLONG: - vshPrint(ctl, "%-15s: %llu\n", params[i].field, params[i].value.ul); - break; - case VIR_TYPED_PARAM_DOUBLE: - vshPrint(ctl, "%-15s: %f\n", params[i].field, params[i].value.d); - break; - case VIR_TYPED_PARAM_BOOLEAN: - vshPrint(ctl, "%-15s: %d\n", params[i].field, params[i].value.b); - break; - default: - vshPrint(ctl, "not implemented scheduler parameter type\n"); - } + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); } } @@ -4935,38 +4916,9 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) } for (i = 0; i < nparams; i++) { - switch (params[i].type) { - case VIR_TYPED_PARAM_INT: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.i); - break; - case VIR_TYPED_PARAM_UINT: - vshPrint(ctl, "%-15s: %u\n", params[i].field, - params[i].value.ui); - break; - case VIR_TYPED_PARAM_LLONG: - vshPrint(ctl, "%-15s: %lld\n", params[i].field, - params[i].value.l); - break; - case VIR_TYPED_PARAM_ULLONG: - vshPrint(ctl, "%-15s: %llu\n", params[i].field, - params[i].value.ul); - break; - case VIR_TYPED_PARAM_DOUBLE: - vshPrint(ctl, "%-15s: %f\n", params[i].field, - params[i].value.d); - break; - case VIR_TYPED_PARAM_BOOLEAN: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.b); - break; - case VIR_TYPED_PARAM_STRING: - vshPrint(ctl, "%-15s: %s\n", params[i].field, - params[i].value.s); - break; - default: - vshPrint(ctl, "unimplemented blkio parameter type\n"); - } + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); } } else { /* set the blkio parameters */ @@ -5112,36 +5064,13 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) } for (i = 0; i < nparams; i++) { - switch (params[i].type) { - case VIR_TYPED_PARAM_INT: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.i); - break; - case VIR_TYPED_PARAM_UINT: - vshPrint(ctl, "%-15s: %u\n", params[i].field, - params[i].value.ui); - break; - case VIR_TYPED_PARAM_LLONG: - vshPrint(ctl, "%-15s: %lld\n", params[i].field, - params[i].value.l); - break; - case VIR_TYPED_PARAM_ULLONG: - if (params[i].value.ul == VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) - vshPrint(ctl, "%-15s: unlimited\n", params[i].field); - else - vshPrint(ctl, "%-15s: %llu kB\n", params[i].field, - params[i].value.ul); - break; - case VIR_TYPED_PARAM_DOUBLE: - vshPrint(ctl, "%-15s: %f\n", params[i].field, - params[i].value.d); - break; - case VIR_TYPED_PARAM_BOOLEAN: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.b); - break; - default: - vshPrint(ctl, "unimplemented memory parameter type\n"); + if (params[i].type == VIR_TYPED_PARAM_ULLONG && + params[i].value.ul == VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) { + vshPrint(ctl, "%-15s: unlimited\n", params[i].field); + } else { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); } } @@ -6562,34 +6491,9 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) } for (i = 0; i < nparams; i++) { - switch(params[i].type) { - case VIR_TYPED_PARAM_INT: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.i); - break; - case VIR_TYPED_PARAM_UINT: - vshPrint(ctl, "%-15s: %u\n", params[i].field, - params[i].value.ui); - break; - case VIR_TYPED_PARAM_LLONG: - vshPrint(ctl, "%-15s: %lld\n", params[i].field, - params[i].value.l); - break; - case VIR_TYPED_PARAM_ULLONG: - vshPrint(ctl, "%-15s: %llu\n", params[i].field, - params[i].value.ul); - break; - case VIR_TYPED_PARAM_DOUBLE: - vshPrint(ctl, "%-15s: %f\n", params[i].field, - params[i].value.d); - break; - case VIR_TYPED_PARAM_BOOLEAN: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.b); - break; - default: - vshPrint(ctl, "unimplemented block I/O throttle parameter type\n"); - } + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); } ret = true; @@ -17086,8 +16990,12 @@ vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) ret = virAsprintf(&str, "%s", item->value.b ? _("yes") : _("no")); break; + case VIR_TYPED_PARAM_STRING: + str = vshStrdup (ctl, item->value.s); + ret = str ? 0 : -1; + default: - vshError(ctl, _("unimplemented block statistics parameter type")); + vshError(ctl, _("unimplemented parameter type %d"), item->type); } if (ret < 0) -- 1.7.7.4
Dňa 20.12.2011 0:08, Eric Blake wrote / napísal(a):
No need to repeat code for formatting typed parameters.
* tools/virsh.c (vshGetTypedParamValue): Support strings. (cmdSchedinfo, cmdBlkiotune, cmdMemtune, cmdBlkdeviotune): Use it for less code. --- tools/virsh.c | 134 +++++++++------------------------------------------------ 1 files changed, 21 insertions(+), 113 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 3654589..a3ec7e9 100644 --- a/tools/virsh.c +++ b/tools/virsh.c
@@ -6562,34 +6491,9 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) }
for (i = 0; i< nparams; i++) { - switch(params[i].type) { - case VIR_TYPED_PARAM_INT: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.i); - break; - case VIR_TYPED_PARAM_UINT: - vshPrint(ctl, "%-15s: %u\n", params[i].field, - params[i].value.ui); - break; - case VIR_TYPED_PARAM_LLONG: - vshPrint(ctl, "%-15s: %lld\n", params[i].field, - params[i].value.l); - break; - case VIR_TYPED_PARAM_ULLONG: - vshPrint(ctl, "%-15s: %llu\n", params[i].field, - params[i].value.ul); - break; - case VIR_TYPED_PARAM_DOUBLE: - vshPrint(ctl, "%-15s: %f\n", params[i].field, - params[i].value.d); - break; - case VIR_TYPED_PARAM_BOOLEAN: - vshPrint(ctl, "%-15s: %d\n", params[i].field, - params[i].value.b); - break; - default: - vshPrint(ctl, "unimplemented block I/O throttle parameter type\n"); - } + char *str = vshGetTypedParamValue(ctl,¶ms[i]); ( in all other instances ) vshGetTypedParamValue uses virAsprintf internaly, so there's a possiblity that it will return NULL as the parameter if an error happens.
Please squash in this fix along with checking for return value in instances you added. This fixes a function that I tampered with previously. The code did not skip to the end, if vshGetTypedParamValue returned NULL. Thanks. diff --git a/tools/virsh.c b/tools/virsh.c index a3ec7e9..fa66579 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1187,7 +1187,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) continue; if (!(value = vshGetTypedParamValue(ctl, par))) - continue; + goto cleanup; /* to print other not supported fields, mark the already printed */ par->field[0] = '\0'; /* set the name to empty string */ @@ -1215,7 +1215,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) continue; if (!(value = vshGetTypedParamValue(ctl, params+i))) - continue; + goto cleanup; vshPrint(ctl, "%s %s %s\n", device, params[i].field, value); VIR_FREE(value); ---------------- (Hopefuly thunderbird will not mangle this )
+ vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); }
ret = true; @@ -17086,8 +16990,12 @@ vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) ret = virAsprintf(&str, "%s", item->value.b ? _("yes") : _("no")); break;
+ case VIR_TYPED_PARAM_STRING: + str = vshStrdup (ctl, item->value.s); + ret = str ? 0 : -1;
If vshStrdup returns, it's always returns a non-NULL pointer, so this check is not necessary.
+ default: - vshError(ctl, _("unimplemented block statistics parameter type")); + vshError(ctl, _("unimplemented parameter type %d"), item->type); }
if (ret< 0)
ACK with the check for return value added. Nice reduction of lines though :) Peter
On 12/19/2011 04:57 PM, Peter Krempa wrote:
No need to repeat code for formatting typed parameters.
* tools/virsh.c (vshGetTypedParamValue): Support strings. (cmdSchedinfo, cmdBlkiotune, cmdMemtune, cmdBlkdeviotune): Use it for less code. --- tools/virsh.c | 134 +++++++++------------------------------------------------ 1 files changed, 21 insertions(+), 113 deletions(-)
+ char *str = vshGetTypedParamValue(ctl,¶ms[i]); ( in all other instances ) vshGetTypedParamValue uses virAsprintf internaly, so there's a
Dňa 20.12.2011 0:08, Eric Blake wrote / napísal(a): possiblity that it will return NULL as the parameter if an error happens.
Why not go one step further - since the only error is OOM, and we already use vshStrdup as an instant exit on OOM, I can just make vshGetTypedParamValue guarantee a non-NULL return (exit on OOM).
Please squash in this fix along with checking for return value in instances you added. This fixes a function that I tampered with previously. The code did not skip to the end, if vshGetTypedParamValue returned NULL. Thanks.
diff --git a/tools/virsh.c b/tools/virsh.c index a3ec7e9..fa66579 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1187,7 +1187,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) continue;
if (!(value = vshGetTypedParamValue(ctl, par))) - continue; + goto cleanup;
By guaranteeing non-NULL, we don't even need the if.
ACK with the check for return value added. Nice reduction of lines though :)
Pushed with even more lines shaved, as well as adding a missing 'break': diff --git i/tools/virsh.c w/tools/virsh.c index a3ec7e9..4f3c9f8 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -387,7 +387,8 @@ static bool vshConnectionUsability(vshControl *ctl, virConnectPtr conn); static virTypedParameterPtr vshFindTypedParamByName(const char *name, virTypedParameterPtr list, int count); -static char *vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item); +static char *vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); static char *editWriteToTempFile (vshControl *ctl, const char *doc); static int editFile (vshControl *ctl, const char *filename); @@ -1186,8 +1187,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) nparams))) continue; - if (!(value = vshGetTypedParamValue(ctl, par))) - continue; + value = vshGetTypedParamValue(ctl, par); /* to print other not supported fields, mark the already printed */ par->field[0] = '\0'; /* set the name to empty string */ @@ -1214,9 +1214,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) if (!*params[i].field) continue; - if (!(value = vshGetTypedParamValue(ctl, params+i))) - continue; - + value = vshGetTypedParamValue(ctl, params+i); vshPrint(ctl, "%s %s %s\n", device, params[i].field, value); VIR_FREE(value); } @@ -16956,15 +16954,14 @@ vshDomainStateReasonToString(int state, int reason) return N_("unknown"); } +/* Return a non-NULL string representation of a typed parameter; exit + * if we are out of memory. */ static char * vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) { int ret = 0; char *str = NULL; - if (!ctl || !item) - return NULL; - switch(item->type) { case VIR_TYPED_PARAM_INT: ret = virAsprintf(&str, "%d", item->value.i); @@ -16991,15 +16988,17 @@ vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) break; case VIR_TYPED_PARAM_STRING: - str = vshStrdup (ctl, item->value.s); - ret = str ? 0 : -1; + str = vshStrdup(ctl, item->value.s); + break; default: vshError(ctl, _("unimplemented parameter type %d"), item->type); } - if (ret < 0) + if (ret < 0) { vshError(ctl, "%s", _("Out of memory")); + exit(EXIT_FAILURE); + } return str; } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa