Re: [Xen-devel] [PATCH V4 09/24] libxl_json: introduce parser functions for builtin types

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, May 13, 2014 at 11:45:04AM +0100, Ian Campbell wrote:
> On Tue, 2014-05-13 at 11:40 +0100, Wei Liu wrote:
> > On Tue, May 13, 2014 at 10:32:57AM +0100, Ian Campbell wrote:
> > > On Tue, 2014-05-13 at 10:19 +0100, Wei Liu wrote:
> > > > On Tue, May 13, 2014 at 09:26:06AM +0100, Ian Campbell wrote:
> > > > > On Mon, 2014-05-12 at 17:43 +0100, Wei Liu wrote:
> > > > > > On Tue, May 06, 2014 at 04:42:20PM +0100, Wei Liu wrote:
> > > > > > [...]
> > > > > > > +
> > > > > > > +int libxl__uint64_parse_json(libxl__gc *gc, const 
> > > > > > > libxl__json_object *o,
> > > > > > > +                             void *p)
> > > > > > > +{
> > > > > > > +    if (!libxl__json_object_is_integer(o) &&
> > > > > > > +        !libxl__json_object_is_number(o))
> > > > > > > +        return ERROR_FAIL;
> > > > > > > +
> > > > > > > +    if (libxl__json_object_is_integer(o)) {
> > > > > > > +        long long i = libxl__json_object_get_integer(o);
> > > > > > > +
> > > > > > > +        if (i < 0)
> > > > > > > +            return ERROR_FAIL;
> > > > > > > +
> > > > > > 
> > > > > > Just found out that current API will generate -1 for some fields 
> > > > > > (say,
> > > > > > video_memkb), so we have to loosen this check.
> > > > > 
> > > > > Is -1 the flag for "default" in these cases?
> > > > > 
> > > > 
> > > > Yes, that's actually ~0 (LIBXL_MEMKB_DEFAULT) in libxl.
> > > > 
> > > > MemKB type is actually uint64 but the generator generates int64 type,
> > > > hence the -1.
> > > 
> > > Is this a separate (latent) bug? What happens for uint64 values >
> > > INT64_MAX but < UINT64_MAX?
> > > 
> > 
> > It's buggy I think. After the transformation (generator -> string ->
> > parser) it gets sign-extended when it really shouldn't.
> > 
> > And this bug affects all fields which have uint64 type. Sigh. This is a
> > limitation in yajl (yajl_gen_integer) not able to generate uint64 value.
> > Parser (json_callback_number) can cope with values that it cannot
> > convert to long long.
> 
> Oh dear. Are we supposed to be using yajl_gen_number then?
> 

I think so. Some more patches to my queue. Looks like this work is never
coming to an end. :-)

And presumably if we are to make this change we also need to update
libxl.h to have LIBXL_JSON_GENERATE_NUMBER-ish? What would you suggest
for the name?

Now looking back to my libxl__uint64_parse_json it's actually not buggy!
Huh.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel