cvs commit: src/sys/conf options src/sys/dev/em if_em.c src/sys/dev/firewire if_fwe.c if_fwip.c src/sys/dev/fxp if_fxp.c src/sys/dev/ixgb if_ixgb.c src/sys/dev/nge if_nge.c src/sys/dev/re if_re.c src/sys/dev/vge if_vge.c src/sys/kern kern_clock.c ...

[Bruce Evans]

On Sat, 8 Oct 2005, Yar Tikhiy wrote:

> On Wed, Oct 05, 2005 at 11:01:47PM +1000, Bruce Evans wrote:
>> On Wed, 5 Oct 2005, Gleb Smirnoff wrote:
>>
>>> glebius     2005-10-05 10:09:17 UTC
>>>
>>> FreeBSD src repository
>>>
>>> Modified files:
>>>   sys/conf             options
>>> ...
>>> Log:
>>> - Don't pollute opt_global.h with DEVICE_POLLING and introduce
>>>   opt_device_polling.h
>>> - Include opt_device_polling.h into appropriate files.
>>> - Embrace with HAVE_KERNEL_OPTION_HEADERS the include in the files that
>>>   can be compiled as loadable modules.
>>>
>>> Reviewed by:    bde
>>
>> Requested too.  Thanks.
>
> According to this scheme, is every opt_*.h inclusion to be wrapped
> in "ifdef HAVE_KERNEL_OPTION_HEADERS" eventually?

Only in the (non-makefile) sources for modules, only if it is possible,
only if it is easier (or no harder) or better.  I hope this is in most
cases for modules.

The main case where it is not possible is when it is reasonable for
users to edit the module makefiles or put defaults for options in the
environment where the module makefiles can pick them up.  Then defaults
shouldn't go in in the sources since it is always unreasonable for users
to have to edit sources to set options.

The main case where it is easier (or no harder) is for options that
apply to many modules (like DEVICE_POLLING in this commit), especially
when the default should be to not configure the option (the case of
an empty fake options header).  Then editing makefiles to change the
default isn't reasonable since lots of files (may) need changing.
Setting options in the environment where module makefiles can pick
them up would be reasonable, but few makefiles support this, and if
many/all did, for many/all options, the ifdef tangle would be much
worse than for HAVE_KERNEL_OPTION_HEADERS.  It is simpler to just
require using the kernel options headers (or maybe an include dir
containing only options headers) to change the defaults for for this
case.  Note that for such options, to use HAVE_KERNEL_OPTIONS_HEADERS,
there must be ifdefs in a large number of source files, and to not use it
there must be fake option header generation in a large number of
makefiles.  It takes about the same amount of code for each method.
but the code is slightly simpler using HAVE_KERNEL_OPTIONS_HEADERS since
for adding a new include of an options header, it doesn't require changing
a separate file (the module makefile) and when adding to options includes
that already use this method it doesn't even require adding the ifdef.

The main cases where it is harder is for modules that have lots of
source with an include of the same options header in each.  Then
using HAVE_KERNEL_OPTION_HEADERS requires duplicating code in more
files.

The main case where it is better is when only one option setting makes
much sense and a fixed setting of it always works.  Then the fixed
setting should be forced in the module sources instead of in the module
makefiles.  The main case of this is INET for network drivers.  Where
module makefiles now do:

%%%
SRCS=	opt_inet.h ...
...
opt_inet.h:
 	echo "#define INET 1" > ${.TARGET}
%%%

and ones with style bugs now do things like:

%%%
SRCS   += opt_inet.h ...
...
opt_inet.h:
 	echo "#define INET 1" > opt_inet.h
%%%

the sources should do:

%%%
#ifdef HAVE_KERNEL_OPTION_HEADERS
#include "opt_inet.h"
...
#endif
...
#if
#if defined(KLD_MODULE) && !defined(HAVE_KERNEL_OPTION_HEADERS)
#define	INET	1
...
#endif
%%%

I don't know if forcing INET on for modules actually works in the very
unusual case that the kernel doesn't have INET, but if it doesn't work
then it doesn't work no matter where INET is forced on.

The similar option INET6 apparently causes problems, or needs very
module-dependent defaults, since it is not alwasy forced on when it
applies and it is controlled by many different knobs:

lmc/Makefile:
 	forced on by bogusly forcing it to 0 (most boolean options including
 	INET6 are tested using #ifdef, not #ifdef, so 0 means "true" except
 	in incosistent tests; for INET6, a quick grep only showed inconsistent
 	tests for INET6 in if_lmc.c and ipsec.c)
if_disc/Makefile:
 	defaulted to off by commenting out its forced setting
if_faith/Makefile:
if_gre/Makefile:
if_stf/Makefile:
if_tun/Makefile:
linux/Makefile:
netgraph/gif/Makefile:
netgraph/iface/Makefile:
sppp/Makefile:
 	forced on
if_gif/Makefile:
pf/Makefile:
if_bridge/Makefile:
 	controlled by NO_INET6
ipfilter/Makefile:
 	controlled by NO_INET6.  This makefile also puts -DUSE_INET6 in
 	CFLAGS if it #define's INET6 as 1 in the options header, so there
 	are potential inconsistencies from USE_INET6 having a larger scope
 	than INET6.
netgraph/fec/Makefile:
 	no options headers.  Puts -DINET -DINET6 in CFLAGS
nfsclient/Makefile:
nfsserver/Makefile:
nfs4client/Makefile:
 	controlled by NFS_INET6 (which defaults to on if it is not set
 	in the environment, unlike for NO_INET6).  The finer control
 	provided by this couldn't reasonably be done in the sources,
 	but it apparently hasn't been used much if at all.  It is not
 	mentioned in examples/etc/make.conf or make.conf(5).  Only
 	NO_INET6 is.  It is also not conditional on !NO_INET6.  INET
 	is misconfigured similarly in these Makefiles.

NO_INET is not used to control INET in any options makefile.

Bruce