On 28 Sep 2016, at 1:28 pm -0400, Andrew Gregory wrote:
On 09/03/16 at 10:14pm, ivy.foster@gmail.com wrote:
From: Ivy Foster <ivy.foster@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