[Xen-devel] Re: [PATCH]: change libxl IDL to export a saner interface for upcoming language bindings

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

On Thu, 2010-09-09 at 14:37 +0100, Ian Campbell wrote:
> On Thu, 2010-09-09 at 14:07 +0100, Gianni Tedesco wrote:
> > Firstly remove an anonymous union in libxl_device_pci structure which
> > was making auto-generating language bindings more complicated than
> > necessary and exporting random bits of low level ABI that libxl that
> > would rather hide anyway. There is a corresponding (untested) change to
> > the ocaml binding which maintains previous ml API.
> > 
> > Also make the libxl_file_reference type a Builtin. This is a 'semantic
> > correctness' issue in that libxl ABI/API won't change. But it makes it
> > so that when the IDL is used to generate language bindings that a
> > file_reference type is not exported.
> 
> You forgot to mention the addition of the Numbers type.

Ah yes

> Otherwise:
> 
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> > -libxl_file_reference = Struct("file_reference",[
> > -    ("path", string, False,
> > -"""Path is always set if the file reference is valid. However if
> > -mapped is true then the actual file may already be unlinked."""),
> > -    ("mapped", integer),
> > -    ("data", void),
> > -    ("size", size_t)], autogenerate_destructor=False)
> > +libxl_file_reference = Builtin("file_reference",
> > +                               autogenerate_destructor=False,
> > +                               
> > destructor_fn="libxl_file_reference_destroy",
> > +                               passby=PASS_BY_REFERENCE)
> 
> autogenerate_destructor=False doesn't do much on a Builtin in any case
> since we never generate destructors for builtins. It should probably
> just be a default in the base type.

Oh right, fair enough.

> i.e. cumulatively on your patch:
> 
> Subject: libxl: Builtin types default to autogenerate_destructor=False
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> diff -r fe1050a60d8f -r c2b17bd27d7d tools/libxl/libxl.idl
> --- a/tools/libxl/libxl.idl   Thu Sep 09 14:32:34 2010 +0100
> +++ b/tools/libxl/libxl.idl   Thu Sep 09 14:34:45 2010 +0100
> @@ -14,6 +14,7 @@ libxl_nic_type = Number("nic_type", name
>  
>  libxl_string_list = Builtin("string_list", 
> destructor_fn="libxl_string_list_destroy", passby=PASS_BY_REFERENCE)
>  libxl_key_value_list = Builtin("key_value_list", 
> destructor_fn="libxl_key_value_list_destroy", passby=PASS_BY_REFERENCE)
> +libxl_file_reference = Builtin("file_reference", 
> destructor_fn="libxl_file_reference_destroy", passby=PASS_BY_REFERENCE)
>  
>  libxl_cpumap = Builtin("cpumap", destructor_fn="free")
>  
> @@ -78,11 +79,6 @@ libxl_domain_create_info = Struct("domai
>      ("poolid",       uint32),
>      ("poolname",     string),
>      ])
> -
> -libxl_file_reference = Builtin("file_reference",
> -                               autogenerate_destructor=False,
> -                               destructor_fn="libxl_file_reference_destroy",
> -                               passby=PASS_BY_REFERENCE)
>  
>  libxl_domain_build_info = Struct("domain_build_info",[
>      ("max_vcpus",       integer),
> diff -r fe1050a60d8f -r c2b17bd27d7d tools/libxl/libxltypes.py
> --- a/tools/libxl/libxltypes.py       Thu Sep 09 14:32:34 2010 +0100
> +++ b/tools/libxl/libxltypes.py       Thu Sep 09 14:34:45 2010 +0100
> @@ -33,6 +33,7 @@ class Builtin(Type):
>      """Builtin type"""
>      def __init__(self, typename, **kwargs):
>          kwargs.setdefault('destructor_fn', None)
> +        kwargs.setdefault('autogenerate_destructor', False)
>          Type.__init__(self, typename, **kwargs)
>  
>  class Number(Builtin):
> 
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel