[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