[Xen-devel] Re: [PATCH V7 6/7] libxl: Introduce JSON parsing stuff.

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:
> We use the yajl parser, but we need to make a tree from the parse result
> to use it outside the parser.
> 
> So this patch include json_object struct that is used to hold the JSON
> data.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  tools/libxl/Makefile         |    5 +-
>  tools/libxl/libxl_internal.h |   95 ++++++++
>  tools/libxl/libxl_json.c     |  505 
> ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 604 insertions(+), 1 deletions(-)
>  create mode 100644 tools/libxl/libxl_json.c
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 839df14..a8aa6d5 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -382,4 +382,99 @@ _hidden int libxl__e820_alloc(libxl_ctx *ctx, uint32_t 
> domid, libxl_domain_confi
[...]
> +static inline bool json_object_is_string(const libxl__json_object *o)
> [...]
> +static inline bool json_object_is_integer(const libxl__json_object *o)
> [...]
> +static inline bool json_object_is_map(const libxl__json_object *o)
> [...]
> +static inline bool json_object_is_array(const libxl__json_object *o)
> [...]
> +static inline const char *json_object_get_string(const libxl__json_object *o)
> [...]
> +static inline flexarray_t *json_object_get_map(const libxl__json_object *o)
> [...]
> +static inline flexarray_t *json_object_get_array(const libxl__json_object *o)
> [...]
> +static inline long json_object_get_integer(const libxl__json_object *o)
> [...]
> +_hidden const libxl__json_object *json_object_get(const char *key,
> +                                          const libxl__json_object *o,
> +                                          libxl__json_node_type 
> expected_type);
> +_hidden void json_object_free(libxl_ctx *ctx, libxl__json_object *obj);

These should all be libxl__json_object_FOO. (just json is ok for static
functions in .c files if you prefer but for functions in headers I think
correct namespacing makes sense).

Internal functions (such as [libxl__]json_object_free) should always
take a libxl__gc and not a libxl_ctx.

> +/* s: is the buffer to parse, libxl__json_parse will advance the pointer the
> + *    part that has not been parsed

It can't advance a "const char *s" (at least not in a way which the
caller can see). Is the comment or the implementation wrong?

> + * *yajl_ctx: is set if the buffer have been whole consume, but the JSON
> + *            structure is not complete.
> + * return NULL in case of error or when the JSON structure is not complete.
> + */
> +_hidden libxl__json_object *libxl__json_parse(libxl__gc *gc,
> +                                              const char *s);
> +
>  #endif
> diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> new file mode 100644
> index 0000000..d0fa5be
> --- /dev/null
> +++ b/tools/libxl/libxl_json.c

I think I reviewed this before so I just did a quick skim.

> +const libxl__json_object *json_object_get(const char *key,
> +                                          const libxl__json_object *o,
> +                                          libxl__json_node_type 
> expected_type)
> +{
> +    flexarray_t *maps = NULL;
> +    int index = 0;
> +
> +    if (json_object_is_map(o)) {

This function only operates on map types? Should it therefore be names
[libxl__]json_map_get?

Is passing a non-map to this function a hard error (e.g. abort())?

I suppose I'll find out in the next patch ;-)

> [...]

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel