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)