On Tue, Feb 21, 2012 at 05:28:22PM -0800, Philip Guenther wrote: > On Tue, 21 Feb 2012, Woodchuck wrote: > > >Description: > > /usr/include/sys/param.h contains a "convenience macro" named > > nitems, which causes user code using nitems in an innocent > > context to fail mysteriously. The bug was spotted during > > compilation of c++ code that uses the FLTK port. > > Whether this is a bug depends, IMO, on how <sys/param.h> is getting pulled > in. >
I've done a bit of RTFMing on this issue, and I'd like to quote from the OpenBSD FAQ, http://www.openbsd.org/faq/ports/guide.html <quote> 2.7 Generic Porting Hints [second bullet] Defining BSD is a bad idea. Try to include sys/param.h. This not only defines BSD, it also gives it a proper value. The right code fragment should look like: #if (defined(__unix__) || defined(unix)) && !defined(USG) #include <sys/param.h> #endif </quote> This seems to place a seal of approval on the routine inclusion of this particular file. Elsewhere in that fine FAQ the practice of tricks with macros and lazily defining externals and symbols rather than including the appropriate headers is condemned, and rightly so. > It's a bug that <netdb.h> unconditionally pulls in <sys/param.h>. It > should not, at least not in a standards conforming compilation mode. If > that's how it's getting pulled into this program, then I agree it's a bug > in the OpenBSD headers. Besides netdb.h, sndio.h and resolv.h include sys/param.h I don't have the statistics, but "nitems" is a very common identifier in all kinds of code. Like retval, flags, mask, noptions, and so forth. And a C++ cliche like "int nitems() { return nitems_;}" is going to trigger that macro. (That was the trigger in this case.) Trust me, a novice is going to be totally flummoxed by the bug, particularly when it is triggered by code that works everywhere else in *n*x land, and is due to a file included 4 or 5 layers deep. The current definition may be "legal" but it is not very "nice". I think it's a problem regardless of how param.h gets incorporated, but will not argue the point. > If the application code itself is #including <sys/param.h> itself, well, > it's getting exactly what it asked for. That file is *not* standardized > and may contain whatever the platform feels like, including a macro named > nitems(). Other drag-ins of "*not* standardized* headers in 5.0: $ find /usr/include -maxdepth 1 -type f | \ xargs -n100 grep -l "#include <sys/" |wc -l 78 and in /usr/X11R6/include/X11: $ find . -type f | xargs -n100 grep "#include <sys/" | wc -l 29 Four files include the dreaded sys/params.h ./InitialI.h ./Xos_r.h ./Xpoll.h ./Xtrans/Xtranssock.c > (Code that pulls in header files "just in case" (or worse, "just because > it's present on this system"!) is Just Plain Wrong.) I agree. TENTATIVE FIX: If I surround the nitems declaration in /usr/include/sys/param.h, and in its mother-file, /usr/src/sys/sys/param.h, like this, then application code is protected. #if !defined(nitems) && (defined(_KERNEL) || defined(_STANDALONE)) #define nitems(_a) (sizeof((_a)) / sizeof((_a)[0])) #endif This, by itself, breaks a certain (small) amount of userland. I discovered that patching a total of four other files under /usr/src allowed a make build, make release, and the same in xenocara without nitems-related errors or warnings. The patch consists of inserting these lines, at one appropriate point in each file, typically immediately after the last #include. #if !defined(nitems) #define nitems(_a) (sizeof((_a)) / sizeof((_a)[0])) #endif The patched files are, relative to /usr/src: (patch of the first kind): ../include/sys/param.h sys/sys/param.h (patch of the second kind) sbin/iked/iked.h sbin/newfs.ext2fs/mke2fs.c usr.sbin/ikectl/ikeca.c usr.sbin/relayd/relayd.h I was able to check this only on i386 architecture. A scrubbed machine was installed with OpenBSD 5.0 STABLE, dating from CVS of Feb 18, 2012. The six patches were applied to freshly expanded src and src/sys archives ftped from ftp.openbsd.org on Jan 8/9. No alteration to xenocara/ was needed. The procedure in man 8 release was followed scrupulously. /etc/mk.conf is nonexistent. Clean environment. No tricksies. The rebuilt kernel and userland were used for a final total redo, for verification. The machine is running fine. I tried to do "make regression-tests" in /usr/src, but got an error cascade; I don't know how to use that. Possible breakage: Unknown ports that rely on the nitems definition from sys/param.h. Mitigating factor: such ports must have been developed on OpenBSD (or some unknown system with the same feature), and would break and be fixed along the lines outlined supra, if ported elsewhere, and the fix is easy. Possible breakage: architectures other than i386. I did look through the whole /usr/src tree, not just relying on make build errors. The _KERNEL and _STANDALONE definitions are supplied from the Makefiles. I am hesitantly hopeful that the patches are adequate. The patch files, expandable under /usr/src, and logs of the make build, release and xenocara's equivalents are available gratis, but I do not know what to do with them and seek guidance. (Apologies to Mr Moerbeek and others for the length, if it offends.) 73 Dave AB3NR
