WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH V7 6/7] libxl: Introduce JSON parsing stuff.
From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Date: Thu, 21 Jul 2011 15:23:11 +0100
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Thu, 21 Jul 2011 07:23:47 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1311242124.32010.96.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1311197072-16003-1-git-send-email-anthony.perard@xxxxxxxxxx> <1311197072-16003-7-git-send-email-anthony.perard@xxxxxxxxxx> <1311242124.32010.96.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
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

<Prev in Thread] Current Thread [Next in Thread>