[libvirt] [PATCH 1/7] libvirt.h: consolidate typed parameter handling

[Daniel Veillard]

On Wed, May 18, 2011 at 07:52:43AM +0200, Matthias Bolte wrote:
> 2011/5/18 Hu Tao <hutao at cn.fujitsu.com>:
> > On Tue, May 17, 2011 at 04:42:09PM -0600, Eric Blake wrote:
> >> * include/libvirt/libvirt.h.in (virTypedParameterType)
> >> (VIR_TYPED_PARAM_FIELD_LENGTH, _virTypedParameter): New enum,
> >> macro, and type.
> >> (virSchedParameter, virBlkioParameter, virMemoryParameter):
> >> Rewrite in terms of a common type, while keeping all old public
> >> names for backwards compatibility.
> >> (struct _virSchedParameter, struct _virBlkioParameter)
> >> (struct _virMemoryParameter): Delete - these are private names.
> >> * python/generator.py (enum): Cope with the refactoring.
> >> ---
> >>  include/libvirt/libvirt.h.in |  144 +++++++++++++++++++++++------------------
> >>  python/generator.py          |   12 ++++
> >>  2 files changed, 93 insertions(+), 63 deletions(-)
> >>
> >> +/**
> >> + * virTypedParameter:
> >> + *
> >> + * A named parameter, including a type and value.
> >> + */
> >> +typedef struct _virTypedParameter virTypedParameter;
> >> +
> >> +struct _virTypedParameter {
> >> +    char field[VIR_TYPED_PARAM_FIELD_LENGTH];  /* parameter name */
> >> +    int type;   /* parameter type, virTypedParameterType */
> >
> > virTypedParameterType type; ?
> 
> We typically use int even for enum values. There is probably a reason
> for that but I'm not aware of it right now.

  enum don't have a fixed size defined by the C standard. So we avoid
using then for any API either in function signature or within public
structure because we can't guarantee that the resulting ABI will be
stable from one compiler to another.

> In this case we might need to stick to int for ABI compatibility,
> because the old structs used int. Assuming that enum is not guaranteed
> to be the same size as int by the C standard, but I don't know.

  yes we must keep int.

> >> +    union {
> >> +        int i;                      /* type is INT */
> >> +        unsigned int ui;            /* type is UINT */
> >> +        long long int l;            /* type is LLONG */
> >> +        unsigned long long int ul;  /* type is ULLONG */
> >> +        double d;                   /* type is DOUBLE */
> >> +        char b;                     /* type is BOOLEAN */
> >> +    } value; /* parameter value */
> >> +};
> >> +
> >> +/**
> >> + * virTypedParameterPtr:
> >> + *
> >> + * a pointer to a virTypedParameter structure.
> >> + */
> >> +typedef virTypedParameter *virTypedParameterPtr;
> >> +
> >> +/* Management of scheduler parameters */
> >> +
> >>  /**
> >>   * virDomainSchedParameterType:
> >>   *
> >>   * A scheduler parameter field type
> >>   */
> >>  typedef enum {
> >> -    VIR_DOMAIN_SCHED_FIELD_INT     = 1, /* integer case */
> >> -    VIR_DOMAIN_SCHED_FIELD_UINT    = 2, /* unsigned integer case */
> >> -    VIR_DOMAIN_SCHED_FIELD_LLONG   = 3, /* long long case */
> >> -    VIR_DOMAIN_SCHED_FIELD_ULLONG  = 4, /* unsigned long long case */
> >> -    VIR_DOMAIN_SCHED_FIELD_DOUBLE  = 5, /* double case */
> >> -    VIR_DOMAIN_SCHED_FIELD_BOOLEAN = 6  /* boolean(character) case */
> >> +    VIR_DOMAIN_SCHED_FIELD_INT     = VIR_TYPED_PARAM_INT,
> >> +    VIR_DOMAIN_SCHED_FIELD_UINT    = VIR_TYPED_PARAM_UINT,
> >> +    VIR_DOMAIN_SCHED_FIELD_LLONG   = VIR_TYPED_PARAM_LLONG,
> >> +    VIR_DOMAIN_SCHED_FIELD_ULLONG  = VIR_TYPED_PARAM_ULLONG,
> >> +    VIR_DOMAIN_SCHED_FIELD_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
> >> +    VIR_DOMAIN_SCHED_FIELD_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
> >>  } virSchedParameterType;
> >
> > Can we remove VIR_DOMAIN_SCHED_FIELD_XXX and use VIR_TYPED_PARAM_XXX
> > directly since parameter types are basically types like int, long, ...
> > and don't depend on what parameters are?
> >
> > Likewise for other PARAMs in this patch.
> 
> We cannot, because the old symbols are part of the released API so we
> cannot remove them without breaking backwards compatibility.

  Agreed.
Patch looks fine to me, and we should do that cleanup before the next
release !

 ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/