[PATCH] remove "ENABLE_" symbols from httpd.c

[Rob Landley]

On Tuesday 11 April 2006 9:47 am, Bernhard Fischer wrote:
> >> actually, i realized that but, if you look closely at that code, it
> >> defined the ENABLE_ macros, then did basically nothing with them. as
>
> The defined ENABLE macros there are for the STANDALONE version of http.
> We currently don't care about the standalone stuff, but i see no reason
> to rip the standalone stuff out as we will need to put it back in
> anyway.

When we do "make standalone", it should work based on menuconfig.  We really 
shouldn't need extra infrastructure to do this, I posted a prototype 
implementation a year ago...

http://www.busybox.net/lists/busybox/2005-March/013939.html

The hard part is getting our makefile to produce the appropriate dependencies, 
which is why I'm reading through the Monster Make Grimoire.

I'll also have to go through our Config.in files and make sure that our 
configuration names are sufficiently standardized, and that our dependency 
checking works right, but in general we should be able to build busybox.links 
and then iterate through that, synthesize a temporary .config (using variant 
of the miniconfig work I've got queued up) and build each applet one at a 
time (or possibly synthesizing a temporary makefile, not sure yet)...

I know what I want to do, and have a half-dozen potential theories about how 
to do it.  Need to get more infrastructure in place to play with, and learn 
more about make...

> >> i read it, the value of the ENABLE_ macros is so that they can be
> >> used in C code but the few instances of actual usage in httpd.c were
> >> *strictly* in preprocessor conditionals, which is already nicely
> >> handled by the CONFIG_ macros.  in short, they had no value
> >> whatsoever.
>
> They were a default config for STANDALONE.

Why would standalone need a separate config?  It's just a packaging choice, 
not a functional one...

> >> my thought is that, for now, just remove them until they can be
> >> re-implemented cleanly and properly.  as it is now, they're
> >> implemented incompletely and have no value.
>
> I agree on the incomplete, but the real "fix" is to use the ENABLE_ and
> remove the CONFIG:

How is hard-coding configuration symbol values into the C code not broken?

I just applied the patch to remove that...

Rob

> 1) prefarably moving to
> if (ENABLE_this && condition) { ....}

Agreed.

> 2) or, in rare cases where we just have to rely on the preprocessor, to
> USE_this(static this_global_var;)

Woot.

> 3) and if nothing of the above is of any help, then
> #if ENABLE_this
> #include "that.h"
> #endif

Only really needed when we're finally ready to remove the CONFIG_ symbols from 
being #included in C code.

Actually, what might be nice is having the CONFIG symbols in a separate 
"oldconfig.h", one that is _not_ #included in libbb.h or busybox.h.  We can 
stick that at the top of any file that still needs it, and remove them one by 
one as we get applets ported over...

> []
>
> >i didn't remove the ENABLE_ stuff because i didn't agree with the
> >direction.  i removed it since it it didn't seem to be doing much, and
> >it would be cleaner to put it back in when it can be done properly.
> >
> >does that make sense?
>
> If you want to play with httpd, then please move it to ENABLE and CC the
> patch to vodz for his comments.
>
> PS: in case this is not clear, please see (or try) the attached patch, do a
> distclean defconfig;make. _That's_ the direction we're heading, with the
> three points in mind that I listed above.

It would be nicer if those symbols got spit out to a separate file, for now.

I'll keep that in mind when I update our menuconfig to the 2.6.16 kernel code.  
(Won't be tonight...)

Rob
-- 
Never bet against the cheap plastic solution.