Re: [Xen-devel] [PATCH V5 19/32] libxl/gentypes.py: don't generate default values

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-20 at 18:17 +0100, Wei Liu wrote:
> On Tue, May 20, 2014 at 02:29:09PM +0100, Ian Campbell wrote:
> > On Tue, 2014-05-13 at 22:54 +0100, Wei Liu wrote:
> > > If a type has init_val defined and a field of the type has the value of
> > > init_val, there's no need to generate output for that field.
> > 
> > Please can you explain why there is no need. And I think you need to
> > mention JSON here somewhere.
> > 
> 
> Sure.
> 
> "If a type has init_val defined and a field of the type has the value of
> init_val, there's no need to generate output for that field in JSON
> output. When the parser consumes that generated JSON object, all default
> values should be automatically filled in."

Ack.

> > > Also define a bunch of init_vals for enumeration types.
> > 
> > I'm not convinced by this. If a type has no initval then you should
> > compare it to zero, which is a valid thing to do even for an enum.
> > 
> 
> What I really did here for enum type was to replace magic number 0 (or
> any other predefined value) with a meaningful macro, so that the
> generated code can have
>  if (FOO == LIBXL_FOO_DEFAULT)
> other than
>  if (FOO == 0)
> which has better readability.

I'm more concerned with the readability of the .idl files than the
generated code. I think having to specify defaults for everything harms
that.

I think if (FOO) and if (!FOO) are perfectly fine in the generated code
for the cases where there is no init_val. Likewise == 0 or != 0.

I wouldn't object to the code noticing that there was no init_val and
doing a lookup for the name given to 0 so it could use it (if it
exists), but that seems like unnecessary work in the code generator.

> > NB you changed the init_val for some typesto (e.g. from 1 to
> > LIBXL_VGA_INTERFACE_TYPE_CIRRUS), which is a no semantic change change,
> > but did mean I had to double check.
> > 
> 
> Corret, I didn't intend to make any semantic changes.

Could you just note in the commit log that there are cases of switching
to the named init_val from a numeric one as well as adding new init_vals
(assuming there is any need to add new ones after hte above comments are
taken into account)

Thanks,
Ian.



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