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']))"