[libvirt] [PATCH] virsh: simply printing of typed parameters

Peter Krempa pkrempa at redhat.com
Mon Dec 19 23:57:45 UTC 2011


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,&params[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




More information about the libvir-list mailing list