Re: [Xen-devel] [PATCH v3-RESEND 09/28] libxl: make the libxl error type an IDL enum

[Rob Hoes <Rob.Hoes@xxxxxxxxxx>]

> On Mon, 2013-10-21 at 14:32 +0100, Rob Hoes wrote:
> > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> > index 16a92a4..11c53cf 100644
> > --- a/tools/libxl/libxl_device.c
> > +++ b/tools/libxl/libxl_device.c
> > @@ -497,7 +497,7 @@ static void multidev_one_callback(libxl__egc *egc,
> > libxl__ao_device *aodev)  {
> >      STATE_AO_GC(aodev->ao);
> >      libxl__multidev *multidev = aodev->multidev;
> > -    int i, error = 0;
> > +    int i, err = 0;
> 
> Why the spurious s/error/err/ ?
> 
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 5c43d6f..778a416 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -28,6 +28,23 @@ MemKB = UInt(64, init_val =
> "LIBXL_MEMKB_DEFAULT")
> > # Constants / Enumerations  #
> >
> > +libxl_error = Enumeration("error", [
> [...]
> > +    ], namespace = "")
> 
> Ah, because you've defined "enum error" as an unnamespaced type.
> Irrespective of the clash you've found I think this is a no go since an
> application might reasonably be using "error" (also, err is a standard
> function too, see err.h and/or err(3)).

Yes, I wanted to make sure that the enum value names did not change. With the 
default namespace, you'd get values such as "LIBXL_ERROR_FAIL" rather than 
"ERROR_FAIL".

> Assuming the idl doesn't support anonymous enums (I don't recall writing
> anything to do that ;-)) and you quite reasonably don't want to add such
> support I think enum libxl_error as the name is fine.

I think the IDL constructs enums as follows, for each value: namespace + "_" + 
enum name + "_" + value name. So naming the enum "libxl_error" again gives 
values such as "LIBXL_ERROR_FAIL", and we're back to square one :)

Perhaps we need another option in the IDL for this?

> I wonder if we should begin transitioning over to using this as the return
> type. Probably mostly involves horrible pain...
> 
> > +
> >  libxl_domain_type = Enumeration("domain_type", [
> >      (-1, "INVALID"),
> >      (1, "HVM"),
> 


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