[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 2/5] virsh: Improve vshCommandOptTimeoutToMs().




On 05/28/2015 05:31 AM, Andrea Bolognani wrote:
> Use vshCommandOptUInt() instead of parsing the value as a signed
> integer and checking whether it's positive afterwards.
> 
> Improve comments as well.
> ---
>  tools/virsh.c | 40 ++++++++++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 865948f..1348985 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1860,31 +1860,39 @@ vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt)
>      return NULL;
>  }
>  
> -/* Parse an optional --timeout parameter in seconds, but store the
> - * value of the timeout in milliseconds.  Return -1 on error, 0 if
> - * no timeout was requested, and 1 if timeout was set.  */
> +/*
> + * vshCommandOptTimeoutToMs:
> + * @ctl virsh control structure
> + * @cmd command reference
> + * @timeout result
> + *
> + * Parse an optional --timeout parameter in seconds, but store the
> + * value of the timeout in milliseconds.
> + * See vshCommandOptInt()
> + */
>  int
>  vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout)
>  {
> -    int rv = vshCommandOptInt(cmd, "timeout", timeout);
> +    int ret;
> +    unsigned int utimeout;
>  
> -    if (rv < 0 || (rv > 0 && *timeout < 1)) {
> +    if ((ret = vshCommandOptUInt(cmd, "timeout", &utimeout)) < 0)

This changes the logic such that utimeout == 0 doesn't get messaged like
it would have previously if *timeout was == 0 (or < 1).


>          vshError(ctl,
>                   _("Numeric value for <%s> option is malformed or out of range"),
>                   "timeout");
> -        return -1;
> -    }
> -    if (rv > 0) {
> -        /* Ensure that we can multiply by 1000 without overflowing. */
> -        if (*timeout > INT_MAX / 1000) {
> -            vshError(ctl, "%s", _("timeout is too big"));
> -            return -1;
> -        }
> -        *timeout *= 1000;
> +    if (ret <= 0)

s/<=/==

It cannot be < due to previous check. So no option, returns 0

> +        return ret;
> +
> +    /* Ensure that we can multiply by 1000 without overflowing. */
> +    if (utimeout > INT_MAX / 1000) {
> +        vshError(ctl, "%s", _("timeout is too big"));

s/big/long

(ironically ;-))

John

> +        ret = -1;
> +    } else {
> +        *timeout = ((int) utimeout) * 1000;
>      }
> -    return rv;
> -}
>  
> +    return ret;
> +}
>  
>  static bool
>  vshConnectionUsability(vshControl *ctl, virConnectPtr conn)
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]