[libvirt] [PATCH] json: Fix *GetBoolean functions

Eric Blake eblake at redhat.com
Thu May 5 19:13:13 UTC 2011


On 05/05/2011 07:10 AM, Jiri Denemark wrote:
> They were not used anywhere so far so nobody noticed they are broken.
> ---
>  src/util/json.c |   11 ++++++-----
>  src/util/json.h |    4 ++--
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/src/util/json.c b/src/util/json.c
> index 0daeae9..df4771d 100644
> --- a/src/util/json.c
> +++ b/src/util/json.c
> @@ -491,12 +491,13 @@ int virJSONValueGetNumberDouble(virJSONValuePtr number, double *value)
>  }
>  
>  
> -int virJSONValueGetBoolean(virJSONValuePtr val)
> +int virJSONValueGetBoolean(virJSONValuePtr val, bool *value)
>  {
> -    if (val->type != VIR_JSON_TYPE_NUMBER)
> +    if (val->type != VIR_JSON_TYPE_BOOLEAN)

This bug was indeed embarrassing.

>          return -1;
>  
> -    return val->data.boolean;
> +    *value = val->data.boolean;
> +    return 0;
>  }

Technically, returning val->data.boolean was okay (0 or 1 is distinct
from -1), but then the user has the burden of checking for -1 before
assigning remaining values back to a bool.

>  int virJSONValueGetNumberLong(virJSONValuePtr object, long long *value);
>  int virJSONValueGetNumberUlong(virJSONValuePtr object, unsigned long long *value);
>  int virJSONValueGetNumberDouble(virJSONValuePtr object, double *value);

And given the precedence of other functions that reserve the return
value for 0/-1 and pass the result by value through the argument pointer,

> -int virJSONValueGetBoolean(virJSONValuePtr object);
> +int virJSONValueGetBoolean(virJSONValuePtr object, bool *value);

I totally agree with this change.

ACK.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110505/6095dad5/attachment-0001.sig>


More information about the libvir-list mailing list