5:08 p.m.
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
5:57 p.m.
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
6:19 p.m.
On 12/19/2011 04:57 PM, Peter Krempa wrote:
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(-)
>
> + 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.
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4721
days inactive
4722
days old
2 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa