[pacman-dev] [PATCH v2 2/2] Make pacman build cleanly with clang's -Wassign-enum
ivy.foster at gmail.com
Sat Oct 1 19:01:53 UTC 2016
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!
More information about the pacman-dev