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
|