[libvirt] beginner project idea: enum scrubbing [was: Fwd: Start contributing with Libvirt (Finding a mentor or helpful tips)]

[Eric Blake]

On 03/31/2014 03:43 PM, Eric Blake wrote:
> On 03/31/2014 10:00 AM, Julio Faracco wrote:
>> Hi everybody!
>>
>> I sent an e-mail to Daniel asking about contributing to the libvirt
>> community for free.
>> So, I copied the e-mail to the libvirt mailing list as Daniel suggested me.
>> If anyone can help me I'd be pleased.
> 
> Welcome to the community.
> 
> I'd suggest starting off by reading http://libvirt.org/hacking.html and
> http://libvirt.org/governance.html to get more of a feel for what
> contributions involve.
> 

Here's an idea for a beginner's project.  Very grunt work, but you
mostly need time and patience, and not much understanding of the code
base (I'd do it myself, if I had the time):

We are inconsistent on how we declare enums.  In our public headers, we
made it a point to do:

typedef enum {
...
} virName;

as it allows clients to use just 'virName' instead of 'enum virName'
when referring to a variable holding an enum value.  However, we also
explicitly use 'int' instead of 'virName' for any API where an enum is
passed in; this is intentional, because some compilers have flags that
can change ABI if you pass enum types (if all values of an enum fit in a
char, then the compiler can expose options to treat it as either a char
or as an int, where the width chosen then affects how other code must
link to that struct/function), whereas using 'int' leaves us with a
stable ABI regardless of whether the client code played with those
compiler flags.  Besides, the C language is fairly lax about conversion
in either direction between int and enum, unlike C++.

Meanwhile, in our internal headers, we often have:

enum virName {
...
};

which means we HAVE to use 'enum virName' when referring to a variable
holding one of those values.  What's more, we have tended to carry over
the public header notion of using 'int' instead of the enum type, so in
util/ and conf/ you'll see lots of headers with struct members such as
'int type; /* enum virName */'.  But this is stupid - our internal
headers are NOT tied by ABI compatibility issues.  And when seeing a
function prototype, foo(int type) is a lot less instructive than
foo(virName type), where type is supposed to be an enum value.  Plus, we
have lately been using the coding style of 'switch ((enum virName)expr)
{...}' with no default: clause, to let the compiler enforce that we've
covered all the enum values; but the cast in that switch is redundant if
expr already has the proper enum type to begin with (and I'm a fan of
removing redundant casts).

I'd really like to see this cleaned up to use the typedef declaration
style throughout our codebase, cleaning up internal headers to directly
use enum types in both struct declarations and function declarations
(where _only_ the public headers are forced to remain with int for ABI
reasons), and checking that switch statements that try to cover all
possible enum values use a consistent style that best aids the compiler.
 Ideally, we'd also implement some syntax check rules to enforce the
style in the future; that part is trickier for a newcomer, but I don't
mind helping on that front (and to some degree, tasks like this are
sometimes easier if you write the checker first then hack the codebase
until the checker no longer warns, then rebase the code into small
enough patches with the checker last).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140402/e2afab00/attachment-0001.sig>