[libvirt] [PATCH RFC 0/2] util: Add VIR_ENUM_IMPL_LABEL
Daniel P. Berrangé
berrange at redhat.com
Fri Jul 27 09:24:11 UTC 2018
On Fri, Jul 27, 2018 at 11:17:13AM +0200, Michal Privoznik wrote:
> On 07/26/2018 07:49 PM, Cole Robinson wrote:
> > This series adds VIR_ENUM_IMPL_LABEL and a demo conversion.
> >
> > VIR_ENUM_IMPL_LABEL allows passing in a string that briefly
> > describes the value the enum represents, which we use to
> > generate error messages for FromString and ToString
> > function failures.
> >
> > This will allow us to drop a lot of code and unique strings
> > that need translating.
> >
> > Patch 2 is an example usage, converting virDomainVirtType
> > enum. It's not a full conversion for this particular enum,
> > just a demo. The enum creation now looks like
> >
> > VIR_ENUM_IMPL_LABEL(virDomainVirt,
> > "domain type",
> > VIR_DOMAIN_VIRT_LAST, ...
> >
> > FromString failure reports this error for value 'zzzz'
> >
> > invalid argument: Unknown 'domain type' value 'zzzz'
> >
> > ToString failure reports this error for unknown type=83
> >
> > internal error: Unknown 'domain type' internal value '83'
> >
> >
> > Seems like a win to me but I'm interested in other opinions.
> > This could also be a good BiteSizedTask for converting existing
> > enums
> >
>
> Agreed.
>
> > Cole Robinson (2):
> > util: Add VIR_ENUM_IMPL_LABEL
> > conf: Convert virDomainVirtType to VIR_ENUM_IMPL_LABEL
> >
> > src/conf/domain_conf.c | 10 ++--------
> > src/util/virutil.c | 20 ++++++++++++++++----
> > src/util/virutil.h | 15 ++++++++++-----
> > 3 files changed, 28 insertions(+), 17 deletions(-)
> >
>
> I like this. You can count on my ACK. But we should probably let others
> chime in and express their preference.
I think its good. My only comment is whether we could come up with a way
to gradually convert each file, while still finishing up with VIR_ENUM_IMPL
as the macro name. A mass rename at the end is one option, but perhaps
theres a more clever approach ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list