[BusyBox] Clean up

Rob Landley rob at landley.net
Wed Jul 27 07:17:55 UTC 2005


On Tuesday 26 July 2005 19:57, Tito wrote:

> Hi,
> Should I try a to write a patch using this solution?
> And if yes for 1.0.1 or 1.1?

A) this is 1.1 material, because changing stuff to actually _use_ this is an 
ongoing cleanup.  (1.1 is going to diverge from 1.0 over time anyway.  This 
is incentive to ship 1.1. in a finite amount of time.)

B) I already added the previous patch making CONFIG_BLAH into 0 or 1 (without 
breaking #ifdef CONFIG_BLAH) so we can use normal if() and dead code 
elimination.

And the _reasons_ I prefer that solution are there's only one thing to 
remember, it's an existing symbol name rather than something new, it's 
completely generic, and it's not brain surgery to understand that "this won't 
happen if this config symbol is switched off, and any modern compiler should 
be smart enough to remove code that can never happen".

With your proposed solution we might potentially have code doing some free 
operation that we haven't got a clean_up_ version for.  (You've got free and 
close, how about munmap?  

The sed code has optional code that traverses a linked list to free a series 
of structures (and things referenced in the structures).  With my preferred 
solution, we can "if(CONFIG) free_function();", make free_function static, 
and life is good.  With yours, we still have to wrap the function's call site 
in #ifdef (because removing just the free() calls may not remove the list 
traversal.)

(When in doubt, go with the generic solution rather than the series of special 
cases.  Aesthetic judgements should point you _towards_ this, not away from 
it.)

That's what I think, anyway.

> Ciao,
> Tito

Rob



More information about the busybox mailing list