libbb/create_icmp6.socket.c: why is entire code conditional?

Jason Schoon floydpink at gmail.com
Sun Mar 26 10:46:50 PST 2006


On 3/26/06, Robert P. J. Day <rpjday at mindspring.com> wrote:
>
>
> (can we avoid top posting, please?)


Why?  That's how my mail client defaults, is there some reason it is wrong?

On Sun, 26 Mar 2006, Jason Schoon wrote:
>
> > I'd vote for #2 in this case, given that the entire file is
> > dependent on a config variable.
> >
> > On 3/26/06, Robert P. J. Day <rpjday at mindspring.com> wrote:
> > >
> > >
> > >   i'm a bit weirded out by the fact that the entirety of the source in
> > > the file libbb/create_icmp6_socket.c is conditionally included by
> > >
> > >   #ifdef CONFIG_FEATURE_IPV6
> > >   ...
> > >   #endif
> > >
> > > that strikes me as unnecessarily obtuse.  it would seem to make more
> > > sense to do one of two things:
> > >
> > > 1) combine both create_icmp_socket.c and create_icmp6_socket.c into a
> > > single source file, and put the conditional inclusion of the IPV6
> > > stuff in *that* file, or
> > >
> > > 2) remove the conditional inclusion and use the typical entry in
> > > Makefile.in of
> > >
> > >   LIBBB-$(CONFIG_FEATURE_IPV6)+= create_icmp6_socket.c
> > >
>
> there are arguments to be made both ways.  if you look in the
> networking/ directory, there are numerous files that conditionally
> include IPv6-related code (ifconfig.c, ifupdown.c, inetd.c,
> interface.c and more).  so there is already a precedent for including
> IPv6 code in the regular source file along with IPv4 stuff.
>
> it would just be nice to have it consistent.
>
> rday


I have no problem with some files having conditional code on this option (I
have submitted patches that have done just that).  However, in the case
where an entire file is wrapped in a config option, we should let the
configuration system handle it, rather than the preprocessor.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://busybox.net/lists/busybox/attachments/20060326/d6f97a10/attachment.htm


More information about the busybox mailing list