Re: [Xen-devel] [PATCH RFC 2/9] libxl_json: introduce libx__object_from_json

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> Given a JSON string, we need to convert it to libxl_FOO struct.
> 
> The approach is:
> JSON string -> libxl__json_object -> libxl_FOO struct
> 
> With this approach we can make use of libxl's infrastructure to do the
> first half (JSON string -> libxl__json_object).
> 
> Second half is done by auto-generated code by libxl's IDL
> infrastructure. IDL patch(es) will come later.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_internal.h |    9 +++++++++
>  tools/libxl/libxl_json.c     |   34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index b3a200d..34b6a26 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1494,6 +1494,15 @@ typedef yajl_gen_status 
> (*libxl__gen_json_callback)(yajl_gen hand, void *);
>  _hidden char *libxl__object_to_json(libxl_ctx *ctx, const char *type,
>                                      libxl__gen_json_callback gen, void *p);
>  
> +typedef struct libxl__json_object libxl__json_object;
> +typedef int (*libxl__parse_json_callback)(libxl_ctx *ctx,
> +                                          libxl__json_object *o,
> +                                          void *p);

libxl__json_parse_callback reads slightly more naturally to me and fits
in with the existing libxl__json_* 

> +_hidden int libxl__object_from_json(libxl_ctx *ctx, const char *type,
> +                                    libxl__parse_json_callback parse,
> +                                    void *p,
> +                                    const char *s);
> +
>    /* holds the CPUID response for a single CPUID leaf
>     * input contains the value of the EAX and ECX register,
>     * and each policy string contains a filter to apply to
> diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> index 3ea56a4..002ae2d 100644
> --- a/tools/libxl/libxl_json.c
> +++ b/tools/libxl/libxl_json.c
> @@ -794,6 +794,40 @@ out:
>      return ret;
>  }
>  
> +int libxl__object_from_json(libxl_ctx *ctx, const char *type,

Internal functions should take libxl__gc *gc and avoid all the GC_INIT,
GC_FREE stuff. You can still access ctx but as CTX (a macro which uses
gc)

> +                            libxl__parse_json_callback parse,
> +                            void *p, const char *s)
> +{
> +    GC_INIT(ctx);
> +    libxl__json_object *o = NULL;

Unnecessary initialiser.

> +    int ret;
> +
> +    o = libxl__json_parse(gc, s);
> +

Drop this blank line to cuddle the error check up with the function.

> +    if (!o) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                   "unable to generate libxl__json_object from JSON 
> representation for %s.",
> +                   type);

You can use the LOG macro here.

I think you want "representation of %s"

> +        ret = -1;

ERROR_FAIL is more normal, but it might depend on the eventual usage.

> +        goto out;
> +    }
> +
> +    ret = parse(ctx, o, p);
> +

Drop this blank.
> +    if (ret) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                   "unable to convert libxl__json_object to %s.",
> +                   type);
> +        ret = -1;

LOG()  and ERROR_FAIL again?

> +        goto out;
> +    }
> +
> +    ret = 0;
> +out:
> +    GC_FREE;
> +    return ret;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel