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 7/7] libxl, Introduce a QMP client

On Thu, 2011-07-21 at 17:29 +0100, Anthony PERARD wrote:
> On Thu, 21 Jul 2011, Ian Campbell wrote:
> 
> > On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:
> > > +static callback_id_pair *qmp_get_callback_from_id(libxl__qmp_handler 
> > > *qmp,
> > > +                                                  const 
> > > libxl__json_object *o)
> > > +{
> > > +    const libxl__json_object *id_object = json_object_get("id", o,
> > > +                                                          JSON_INTEGER);
> > > +    int id = -1;
> > > +    callback_id_pair *pp = NULL;
> > > +
> > > +    if (id_object) {
> > > +        id = json_object_get_integer(id_object);
> >
> > Check that it is an integer? Presumably the -1 error return is never a
> > valid id but it'd save a useless list walk.
> 
> Actually, the parametter JSON_INTEGER given to json_object_get ask to
> explicitly return a json_object with an integer, otherwise, the function
> return null.
> 
> So the get_integer will not return an error.

Oh right, makes sense.

> > > +        SIMPLEQ_FOREACH(pp, &qmp->callback_list, next) {
> > > +            if (pp->id == id) {
> > > +                return pp;
> > > +            }
> > > +        }
> > > +    }
> > > +    return NULL;
> > > +}
> > > +
> > [...]
> > > +static inline yajl_gen_status libxl__yajl_gen_asciiz(yajl_gen hand,
> > > +                                                     const char *str)
> > > +{
> > > +    return yajl_gen_string(hand, (const unsigned char *)str, 
> > > strlen(str));
> > > +}
> >
> > Belongs in libxl_json I think.
> 
> Actually, I have put it here to not expose the yajl_gen.h to all user of
> libxl_internal.h.
> 
> Otherwise, yes, it does not really belong to libxl_qmp.

I wanted it there because my json generating patch also needs it ;-)

I think exposing yajl_gen.h to libxl_internal is ok, it's libxl.h where
we should avoid it.

I can patch this up in my series which uses JSON though so no worries.

Ian.


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