Re: [Xen-devel] [PATCH 05 of 11] libxl: add a new Array type to the IDL

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-06-26 at 17:28 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("[Xen-devel] [PATCH 05 of 11] libxl: add a new Array 
> type to the IDL"):
> > And make all the required infrastructure updates to enable this.
> 
> > diff --git a/tools/libxl/idl.txt b/tools/libxl/idl.txt
> > --- a/tools/libxl/idl.txt
> > +++ b/tools/libxl/idl.txt
> > @@ -145,12 +145,24 @@ idl.KeyedUnion
> >  
> >   A subclass of idl.Aggregate which represents the C union type
> >   where the currently valid member of the union can be determined based
> > - upon another member in the containing type.
> > + upon another member in the containing type. An idl.KeyedUnion must
> > + always be a member of a containing idl.Aggregate type.
> >  
> > - The KeyedUnion.keyvar contains an idl.type the member of the
> > + The KeyedUnion.keyvar contains an idl.Type the member of the
> >   containing type which determines the valid member of the union. The
> >   must be an instance of the Enumeration type.
> 
> Something seems to have been mangled here in this sentence (although
> all your patch does is change the case of the name).
> 
> > +idl.Array
> > +
> > +  A class representing an array or similar elements. An idl.Array must
>                                    ^^
> of.
> 
> > +  always be an idl.Field of a containing idl.Aggregate.
> > +
> > +  idl.Array.elem_type contains an idl.Type which is the type of all
> > +  elements of the array.
> 
> More natural in English would be, I think, "... the type of each
> element of the array".
> 
> > +  idl.Array.len_var contains an idl.Field which is added to the parent
> > +  idl.Aggregate and will contain the length of the array.
> 
> I think you should explicitly say that len_var MUST be a field named
> "num_XXX" where the original field is named XXX, as previously stated.
> 
> In the future this will probably be enforced by the idl compiler.
> 
> 
> Aside from that,
> 
> Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Thanks.

Dario, since this is my patch do you want me to address the above and
resend?

> 
> 
> Thanks,
> Ian.



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