
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