[pacman-dev] [PATCH v2 2/2] Make pacman build cleanly with clang's -Wassign-enum

[Ivy Foster]

On 28 Sep 2016, at  1:28 pm -0400, Andrew Gregory wrote:
> On 09/03/16 at 10:14pm, ivy.foster at gmail.com wrote:
> > From: Ivy Foster <ivy.foster at gmail.com>
> > 
> > In many cases, it was enough to add error or "none" values to enums,
> > to account for cases where functions returned either an enumerated
> > value or 0/-1.
> > 
> > A common code pattern in pacman is to use enums to create bitfields
> > representing various option combinations. Since they are not
> > technically part of the enumerated type used to build them, these
> > bitfields have been reassigned to type int.

> Even though all of these changes are related our enum handling, there
> is a lot going on in this patch.  I would prefer if it were broken up
> into more specific change sets.

Fair enough! I'll start working on that tonight.

> * converting bitfield values to int
>
> I think int is the correct type for bitfields, but it looks like you
> missed many.  Enums like siglevel_t and usage_t are used exclusively
> in the construction of bitfields.  I don't think we should really have
> any variables of those types anywhere.

That makes sense. I like the consistency, too. I'll do a more
fine-grained search for those for the bitfield-to-int patch.

> * adding ERR_OK
> 
> OK - libarchive and libcurl do this and I think it makes sense.
> 
> * adding NONE/ERROR to other enums
> 
> The addition of NONE values to enums appears to be unnecessary to me.
> I only see them being used as empty bitfield values, and, once we
> change those to int, using a literal 0 for those is just fine.

> I'm not fond of adding ERROR values to every enum just so that we can
> return -1 for invalid input.  Most of them look unnecessary because
> the relevant functions are actually returning bitfields and should be
> changed to return an int, making -1 a valid return value.

Cool, I'll do it that way instead.

Thanks for the feedback!

Ivy