[Xen-devel] Re: [PATCH V7 7/7] libxl, Introduce a QMP client

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

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