[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add support for YAJL version 2 API/ABI



On 05/03/2011 11:12 AM, Daniel P. Berrange wrote:
> Version 2.0.0 or yajl changed API. It is fairly trivial for us to
> cope with both APIs in libvirt, so adapt.
> 
> * configure.ac: Probe for yajl2 API
> * src/util/json.c: Conditional support for yajl2 API
> +++ b/src/util/json.c
> @@ -709,7 +709,12 @@ static int virJSONParserHandleBoolean(void * ctx, int boolean_)
>  
>  static int virJSONParserHandleNumber(void * ctx,
>                                       const char * s,

As long as you're touching this, can we use the preferred format of
'void *ctx' and 'const char *s'?

> -                                     unsigned int l)
> +#ifdef HAVE_YAJL2
> +                                     size_t l
> +#else
> +                                     unsigned int l
> +#endif
> +    )

I'm not a fan of #ifdefs inside a function declaration.  Can we instead do:

#ifdef HAVE_YAJL2
typedef size_t yajl_size;
#else
typedef unsigned int yajl_size;
#endif

static int
virJSONParserHandleNumber(void *ctx,
                          const char *s,
                          yajl_size l);

Those typedefs would then be useful for all three function signatures
touched in this file.

> @@ -894,14 +909,22 @@ static const yajl_callbacks parserCallbacks = {
>  /* XXX add an incremental streaming parser - yajl trivially supports it */
>  virJSONValuePtr virJSONValueFromString(const char *jsonstring)
>  {
> -    yajl_parser_config cfg = { 1, 1 };
>      yajl_handle hand;
>      virJSONParser parser = { NULL, NULL, 0 };
>      virJSONValuePtr ret = NULL;
> +#ifndef HAVE_YAJL2
> +    yajl_parser_config cfg = { 1, 1 };
> +#endif
>  
>      VIR_DEBUG("string=%s", jsonstring);
>  
> +#ifdef HAVE_YAJL2
> +    hand = yajl_alloc(&parserCallbacks, NULL, &parser);
> +    yajl_config(hand, yajl_allow_comments, 1);

Does yajl_config properly handle the case where hand is NULL from the
earlier yajl_alloc, or do you need error checking here?

> @@ -1002,15 +1025,26 @@ static int virJSONValueToStringOne(virJSONValuePtr object,
>  
>  char *virJSONValueToString(virJSONValuePtr object)
>  {
> -    yajl_gen_config conf = { 0, " " }; /* Turns off pretty printing since QEMU can't cope */
>      yajl_gen g;
>      const unsigned char *str;
>      char *ret = NULL;
> +#ifdef HAVE_YAJL2
> +    size_t len;
> +#else
>      unsigned int len;

Again, this part would benefit from the conditional typedefs.

> +    yajl_gen_config conf = { 0, " " }; /* Turns off pretty printing since QEMU can't cope */
> +#endif
>  
>      VIR_DEBUG("object=%p", object);
>  
> +#ifdef HAVE_YAJL2
> +    g = yajl_gen_alloc(NULL);
> +    yajl_gen_config(g, yajl_gen_beautify, 0);

Again, any error handling needed?

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]