On Mon, Apr 28, 2014 at 12:16:02AM +0800, Amos Kong wrote:
> String "-3" will be wrongly converted to -34 by strtosz_suffix().
> strtosz_suffix_unit() only returns integer in success situation.
>
> Signed-off-by: Amos Kong <ak...@redhat.com>
Looks correct to me.
nitpick: while not introduced by this patch,
it's cleaner to do error handling in the
if statement rather than handle success specially.
Also if (x) is nicer than if (x != 0).
So:
if (val < 0 || *endptr) {
error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
"a size value representible as a non-negative int64");
return;
}
*obj = val;
processed(ov, name);
this will make this use of strtosz_suffix consistent with all
other uses.
I'm fine with changing this in a follow-up patch though, so:
Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> qapi/opts-visitor.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 5d830a2..881e1b9 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -472,7 +472,7 @@ opts_type_size(Visitor *v, uint64_t *obj, const char
> *name, Error **errp)
>
> val = strtosz_suffix(opt->str ? opt->str : "", &endptr,
> STRTOSZ_DEFSUFFIX_B);
> - if (val != -1 && *endptr == '\0') {
> + if (val >= 0 && *endptr == '\0') {
> *obj = val;
> processed(ov, name);
> return;
> --
> 1.9.0
>
