diff mbox

qapi: treat all negative return of strtosz_suffix() as error

Message ID 1398615362-21654-1-git-send-email-akong@redhat.com
State New
Headers show

Commit Message

Amos Kong April 27, 2014, 4:16 p.m. UTC
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 <akong@redhat.com>
---
 qapi/opts-visitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael S. Tsirkin April 28, 2014, 4:23 a.m. UTC | #1
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 <akong@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 <mst@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
>
diff mbox

Patch

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;