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

2011/5/18 Hu Tao <hutao(a)cn.fujitsu.com&gt;: > 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.

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.

>> +    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.