On 1/23/07, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On 1/23/07, Johannes Weiner <hannes@saeurebad.de> wrote:
Hi,
On Fri, Jan 19, 2007 at 09:54:16PM -0500, Dan McGee wrote:
This mainly deals with code clarity- removing currently unneeded optimizations in order to make the code much more readable and type-checkable. Every enum in the library now has it's own type that should be used instead of the generic 'unsigned char'. In addition, several #define statements dealing with constants were converted to enums.
Uh.. what do you need all this types for...?
There are currently 30 custom types over the lib (IIGC). Does this really bring more clarity?
Do you need to typecheck an error-level?
Actually, yes. Using enums instead of defined constants and arbitrary types brings alot of clarity. Take this instance:
int foo(unsigned int foo_type);
What are the bounds for foo_type? Where do you find the documentation for it? Is it arbitrary? Why is it and int? Why is it unsigned?
int foo(enum foo_type_t type)
This way you know it is an enum and you know that the values must belong to that enum, and thus how/where to find the possible bounds of the call.
Which looks clearer here? foo(20); //20 is purple foo(footype_purple);
I'll add that it helped track down some switch statements that didn't correctly switch on all values of an enum. P.S. Aaron, I'm currently segfaulting on an -Ss operation with this patch. It only seems to do this after both your list patch and my enum patch are applied, when applied separate it worked fine. The problem is definitely in the library side, but the debug output isn't clear enough beyond that. I'm trying to look into it without a whole lot of success yet, let me know if you have the same problem.