On Mon, Feb 20, 2012 at 10:05:02AM -0700, Eric Blake wrote:
> On 02/20/2012 03:15 AM, Daniel Veillard wrote:
> > On Mon, Feb 20, 2012 at 09:59:23AM +0100, Jiri Denemark wrote:
> >> On Mon, Feb 20, 2012 at 11:26:50 +0800, Daniel Veillard wrote:
> >>> diff --git a/src/util/virfile.h b/src/util/virfile.h
> >>> index ec1e90b..184677c 100644
> >>> --- a/src/util/virfile.h
> >>> +++ b/src/util/virfile.h
> >>> @@ -58,10 +58,10 @@ typedef virFileWrapperFd *virFileWrapperFdPtr;
> >>>
> >>> int virFileDirectFdFlag(void);
> >>>
> >>> -enum {
> >>> +enum virFileWrapperFdFlags {
> >>> VIR_FILE_WRAPPER_BYPASS_CACHE = (1 << 0),
> >>> VIR_FILE_WRAPPER_NON_BLOCKING = (1 << 1),
> >>> -} virFileWrapperFdFlags;
> >>> +};
> >>
> >> Actually the error was missing typedef (again, shame on me) but this patch
> >> fixes it as well and since we use these flags OR-ed, it's unlikely we will
> >> ever need to use the type anywhere.
> >
> > Well let's make the typedef explicit, that's cleaner, ACK in advance
> > if you want to make that patch :-)
>
> Is this enough of a preferred style that we should go ahead and add a
> note in HACKING about the style preference, as well as add a syntax
> check and convert the offenders?
>
> Overall, we currently favor raw enums over typedefs:
>
> $ git grep '^enum' | wc
> 518 1226 25319
> $ git grep '^typedef enum' | wc
> 140 488 6520
>
> But those numbers are misleading; in all of our public headers:
>
> $ git grep '^enum' include/ |wc
> 0 0 0
> $ git grep '^typedef enum' include/ |wc
> 79 237 3475
>
> I decided not to push a patch adding the typedef, after all, at least
> not until we decide whether it makes a difference style wise.
I would be slightly inclined to go with typedef, to make it explicit.
I think in this area a lot of new enums are implemented by looking
around what we have and we already have quite a bit of mechanics in
place especially around the conversion from name to values, so I think
cut/paste/modify is frequent for those and if we add the typedef to
all definition we are likely to see them on new code, and that will
avoid that problem that we detect late :-)
so a small +1 on the idea but noth radical :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list