Date: Tue, 16 Mar 2010 02:06:55 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Alexander Best <alexbestms@wwu.de> Cc: freebsd-bugs@FreeBSD.org, freebsd-gnats-submit@FreeBSD.org Subject: Re: misc/144749: [libstand] [patch] make assert.c use abort(3) instead of exit(3) Message-ID: <20100316013735.R24698@delplex.bde.org> In-Reply-To: <201003142330.o2ENUimQ065607@www.freebsd.org> References: <201003142330.o2ENUimQ065607@www.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 14 Mar 2010, Alexander Best wrote: >> Description: > gcc complaints about the use of exit() in assert.c: > > /usr/src/lib/libstand/assert.c:43: warning: implicit declaration of function > 'exit' It's not clear where this should be declared, or even it must exist. exit() might be supplied by the application, but if not then it doesn't exist and it is invalid for assert() to call it. libstand(3) of course doesn't document either exit() or the need to supply it to support assert(). > the attached patch replaces exit() (which is used illegally anyway, because no integer argument is being supplied) with abort(). The use might be legal, since libstand is freestanding so its exit might be unrelated to the Standard C exit. However, most or all implementations of exit() in /sys/boot take an arg. > assert.c uses stand.h which conflicts with stdlib.h. so this patch also defines abort() in stand.h being an external function. abort() certainly is not required to exist, and in fact doesn't exist for any application in /sys/boot, so this change would mainly break most boot code at link time. libstand is independent of libc, so it is a bug for anything in it or any application that uses it to include <stdlib.h>. In libstand and /sys/boot, this bug seems to be present only in ficl (the include is ifdefed in only some places in ficl but in all places in libstand). A nearby function that doesn't exist is atexit(). This is only called from ifdefed out code in zalloc_malloc.c. > > cheers. > alex >> How-To-Repeat: > cd /usr/src/lib/libstand && make >> Fix: > > > Patch attached with submission follows: > > Index: lib/libstand/assert.c > =================================================================== > --- lib/libstand/assert.c (revision 205121) > +++ lib/libstand/assert.c (working copy) > @@ -40,5 +40,5 @@ > else > printf("Assertion failed: (%s), function %s, file %s, line " > "%d.\n", expression, func, file, line); It would be nice to fix the style bug in this (use of C90 string concatenation to obfuscate the message). This style bug os missing in the libc version. > - exit(); > + abort(); > } > Index: lib/libstand/stand.h > =================================================================== > --- lib/libstand/stand.h (revision 205121) > +++ lib/libstand/stand.h (working copy) > @@ -265,6 +265,7 @@ > extern char *optarg; /* getopt(3) external variables */ > extern int optind, opterr, optopt, optreset; > extern int getopt(int, char * const [], const char *); > +extern void abort(void); Further unsorting of the declarations. Note that exit() is of a different nature than getopt() -- it is supplied by the application (if it exists), while getopt() is supplied by libstand. So there should be a separate section to declare exit() and any other function that must be supplied by the application. > > /* pager.c */ > extern void pager_open(void); ISTR that there were once similar problems declaring main(). When it wasn't declared explicitly, gcc -Wmumble used to warn about this. So someone "fixed" this by declaring it in a standard header. But it cannot be declared in a standard header since it has more than one valid parameter list ((void), and the usual (argc, argv), and maybe more). Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100316013735.R24698>