[BusyBox] pre-commit feedback, please

Shaun Jackman sjackman at gmail.com
Fri Jul 29 15:34:13 UTC 2005


On 7/29/05, Rob Landley <rob at landley.net> wrote:
> Yup.  But if we're going to do this at all, we should do it right.  Don't add
> one more special case, and do _NOT_ #define fork() something_that_isn't_fork.
> We should call bb_fork and friends everywhere and let the logic in there
> figure out whether it needs vfork or longjmp based on menuconfig stuff, and
> rip out all the current horrible #ifdef tests that check and see if we need
> to fork or vfork.
> 
> Yeah, it's a more intrusive patch, but it's the right thing to do.  If you
> want to "#define fork() break_the_build=42;" for debugging purposes, fine,
> but don't check that in.

I entirely agree with this. A clean bb_fork() implementation is the right thing.
 
> P.S.  the test in libbb.h, "#if CONFIG_FEATURE..." won't work if it's
> disabled, will it?  Undefined symbol.  That's not an #ifdef.  So this patch
> couldn't go in as-is anyway...

#if xxx works if xxx is undefined (false), defined to 0 (false), or
defined to 1 (true). It does not work if xxx is defined to the empty
string. We currently use undefined for false, and defined to 1 for
true, so it will work.

Cheers,
Shaun



More information about the busybox mailing list