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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, 21 Jul 2011, Ian Campbell wrote:

> 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.

Ok, I will change that.

> > +/* 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?

The comment is wrong, not updated. I'll just remove it as I think the
prototype is explicit enough. (Parse *s, return a json_object OR NULL in
case of error)

> > + * *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?

Yes, the only type to use keys. So, I'll rename it.

> Is passing a non-map to this function a hard error (e.g. abort())?
>
> I suppose I'll find out in the next patch ;-)

It is the same error as if the map does not contain the key or the type
of the object found is not the right one. So not a hard error here.

Thanks,

-- 
Anthony PERARD

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