Date: Sun, 9 Oct 2005 03:20:13 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Yar Tikhiy <yar@comp.chem.msu.su> Cc: cvs-src@freebsd.org, Gleb Smirnoff <glebius@freebsd.org>, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: 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 ... Message-ID: <20051009013251.Y61630@delplex.bde.org> In-Reply-To: <20051008044623.GA2392@comp.chem.msu.su> References: <200510051009.j95A9HpC024040@repoman.freebsd.org> <20051005230120.B51543@delplex.bde.org> <20051008044623.GA2392@comp.chem.msu.su>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20051009013251.Y61630>