[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