[libvirt] [PATCH 1/7] virsh: Make vshCommandOpt* report error

[Ján Tomko]

On 04/02/2014 02:43 PM, Michal Privoznik wrote:
> Currently, the virsh code is plenty of the following pattern:
> 
>   if (vshCommandOptUInt(..) < 0) {
>       vshError(...);
>       goto cleanup;
>   }
> 
> It doesn't make much sense to repeat the code everywhere.
> Moreover, some functions from the family already report
> error some of them don't.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  tests/vcpupin |  1 +
>  tools/virsh.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++------

vshCommandOptTimeoutToMs also calls vshCommandOptInt

>  tools/virsh.h |  2 +-
>  3 files changed, 67 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/vcpupin b/tests/vcpupin
> index f1fb038..a616216 100755
> --- a/tests/vcpupin
> +++ b/tests/vcpupin
> @@ -34,6 +34,7 @@ fail=0
>  $abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 > out 2>&1
>  test $? = 1 || fail=1
>  cat <<\EOF > exp || fail=1
> +error: Unable to parse integer parameter to --vcpu
>  error: vcpupin: Invalid or missing vCPU number.
>  
>  EOF
> diff --git a/tools/virsh.c b/tools/virsh.c
> index f2e4c4a..1371abb 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1489,8 +1489,18 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
>      if (ret <= 0)
>          return ret;
>  
> -    if (virStrToLong_i(arg->data, NULL, 10, value) < 0)
> +    if (virStrToLong_i(arg->data, NULL, 10, value) < 0) {
> +        if (arg->def->flags & VSH_OFLAG_REQ) {
> +            vshError(NULL,

ctl should be used for reporting errors, just like in vshCommandOptStringReq.

> +                     _("missing --%s parameter value"),
> +                     name);

Missing values are caught in vshCommandParse. The error should be the same
regardless of VSH_OFLAG_REQ.

> +        } else {
> +            vshError(NULL,
> +                     _("Unable to parse integer parameter to --%s"),
> +                     name);

>  
> @@ -1692,9 +1742,17 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char *name,
>      ret = vshCommandOptString(cmd, name, &str);
>      if (ret <= 0)
>          return ret;
> -    if (virStrToLong_ull(str, &end, 10, value) < 0 ||
> -        virScaleInteger(value, end, scale, max) < 0)
> +    if (virStrToLong_ull(str, &end, 10, value) < 0) {
> +        vshError(NULL,
> +                 _("Unable to parse integer parameter to --%s"),
> +                 name);
>          return -1;
> +    }
> +
> +    if (virScaleInteger(value, end, scale, max) < 0) {
> +        /* Error reported by the helper. */

Needs vshReportError to propagate the error.

> +        return -1;
> +    }
>      return 1;
>  }
>  
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 3e0251b..6eb17b3 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -168,7 +168,7 @@ struct _vshCmdInfo {
>  struct _vshCmdOptDef {
>      const char *name;           /* the name of option, or NULL for list end */
>      vshCmdOptType type;         /* option type */
> -    unsigned int flags;         /* flags */
> +    unsigned int flags;         /* bitwise OR of VSH_OFLAG_**/
>      const char *help;           /* non-NULL help string; or for VSH_OT_ALIAS
>                                   * the name of a later public option */
>      vshCompleter completer;         /* option completer */
> 

You can push this hunk separately as trivial.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140404/8dbfa0f8/attachment-0001.sig>