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

[Andrea Bolognani]

On Sun, 2015-05-31 at 15:09 -0400, John Ferlan wrote:

> >  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).

My bad. I've added a bunch of test cases to v4 so that something like
this is unlikely to slip through the cracks again.

> >          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

This is actually correct: if the value returned by vshCommandOptUInt()
is negative, an error is reported; an early return is then performed if
the value was <= 0, which means either an error, or no value provided
for an non-mandatory option.

> > +        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 ;-))

I've changed it to report the common error message instead, because this
is really an out-of-range condition. Same for zero. If you feel like a
more specific error message is warranted here we can definitely do that,
I don't feel strongly either way :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team
$ python -c "print('a'.join(['', 'bologn', '@redh', 't.com']))"