Re: [Xen-devel] [PATCH 2/3] libxl_types.idl: use empty Struct for invalid domain type

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-04-10 at 14:00 +0100, Wei Liu wrote:
> On Thu, Apr 10, 2014 at 01:45:15PM +0100, Ian Campbell wrote:
> > On Thu, 2014-04-10 at 13:43 +0100, Wei Liu wrote:
> > > Using None makes gentypes.py generates a C function which generates
> > > nothing. Eventually the generated JSON string is malformed.
> > 
> > I'd prefer to fix the generator to not do this rather than muddying the
> > input description. How hard would it be?
> > 
> 
> It certainly yields a patch much larger than this one-liner. The way to
> fix generator is when parsing keyed-union we need to 1) get hold of the
> key, 2) get its "invalid" value, 3) skip code generation if key points
> to "invalid".

Can you point me to the generate code which wrong? What I'm seeing in
e.g. libxl_domain_build_info_gen_json is:
    s = yajl_gen_map_open(hand);
        ...
    switch (p->type) {
    case LIBXL_DOMAIN_TYPE_HVM:
        s = yajl_gen_map_open(hand);
                ...
        s = yajl_gen_map_close(hand);
        if (s != yajl_gen_status_ok)
            goto out;
        break;
    case LIBXL_DOMAIN_TYPE_PV:
        s = yajl_gen_map_open(hand);
                ...
        s = yajl_gen_map_close(hand);
        if (s != yajl_gen_status_ok)
            goto out;
        break;
    case LIBXL_DOMAIN_TYPE_INVALID:
        break;
    }
    s = yajl_gen_map_close(hand);

Does that produce invalid json somehow?

And if it did could we not detect the use of None with an explicit check
in the generator somewhere rather than worrying about whether a given
enum value is valid or invalid? Relying on INVALID would be wrong
anyway. It is in theory legitimate for a valid value to have no extra
data in the enum -- a bunch of the entries in libxl_event could be so
for example.

> We don't have definition in IDL for "invalid" value yet, so we need to
> add more code to get 2) working.
> 
> I only discovered this when I run testidl. There's no existing users of
> the generation function in libxl, so I think this fix is quite safe.

At least libxl_domain_config_gen_json is used (by xl).

Ian.


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