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