On 28 Sep 2016, at 1:28 pm -0400, Andrew Gregory wrote: > On 09/03/16 at 10:14pm, [email protected] wrote: > > From: Ivy Foster <[email protected]> > > > > 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
