[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

However, there is other instances that some field has uint64 type and
actually generates -1 value.

The discussion below is orthogonal to the problem I found.

> I wonder -- should we omit fields which are set to default from the json
> altogether rather than encode them as some specific value? (the actual
> value is notionally somewhat libxl internal). This would be a case of

LIBXL_MEMKB_DEFAULT is in libxl.h, as with some other default
values, so that the value is actually visible to external users. Not
saying that the actual value affects that much, just to clarify.
There're some defaults that are not visible to external world though.

> comparing them against their IDL init_val in the gen_json function I
> think.
> 

Or the other way around, export all default values to libxl.h so that
libxl users can interpret them as well.

Not sure what's the best approach.

Wei.

> Thoughts?
> 
> Ian.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.