>Number:         7592
>Category:       bin
>Synopsis:       -Wformat err() 'fixes' broke lots of programs.
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 16 23:20:00 1999
>Last-Modified:
>Originator:     Chris G. Demetriou
>Organization:
Chris Demetriou - cgd@netbsd.org - http://www.netbsd.org/People/Pages/cgd.html
Disclaimer: Not speaking for NetBSD, just expressing my own opinion.
>Release:        NetBSD 1.3.x, 1.4, -current
>Environment:
System: NetBSD 1.3.2+ (SPEEDY) #17: Sun Nov 29 16:32:25 PST 1998     cgd@xxx:/a/users/cgd/proj/netbsd/src-1-3-branch/sys/arch/i386/compile/SPEEDY
And inspection of 1.4 and -current sources.

>Description:
	[ I'd noticed this brokenness a ... long time ago, but only recently
	had time to look into it.  to be quite honest, i'm fairly shocked
	and dismayed. ]

	Many programs' error reporting behaviour is broken.  For instance,
	when mount_cd9660's mount() request fails, it spits out messages like:

	68 [speedy] log % mount -t cd9660 foo bar
	mount_cd9660: : No such file or directory

	note the extra " :".

	This is cause by incorrect use of err(), in which it is invoked
	like:

		err(1, "%s", "");

	This code is broken.  To do what it is trying to do, the correct
	invocation of err() is:

		err(1, NULL);

	However, in order to 'fix' gcc -Wformat 'problems,' certain people
	took it upon themselves to break the code, either by writing
	broken code to begin with or, worse, converting from the correct
	call form to the incorrect form to silence warnings!

	As of a mid-April -current (trunk) source tree, there were
	52 instances of broken calls to err() in 37 files.  They were
	found with a:

	find . -name "*.[ch]" | xargs grep 'err(1, "%s", "")'

	over the source tree.  (Now that I think about it, there may be
	problems w/ calls to err() with other exit codes.)

	Using 'cvs annotate' shows that the lines containing the bogus
	calls were touched by the following people with the following
	frequencies:

	28	christos
	14	lukem
	6	mrg
	1	wsanchez, mark, kleink, fair

	(I would guess that the first three folks made a lot of 'fixes',
	and that the rest saw the bad example and copied it in new code,
	but I cannot be sure.)

>How-To-Repeat:
	Run commands or examine sources and function documentation
	as described in the Description section.

>Fix:
	(1) revert all calls of err() like:
		err(1, "%s", "")
	to:
		err(1, NULL)

	(2) If printf-like format checking is desired on {err,warn}{,x}()
	calls, make gcc aware that in some circumstances NULL as the format
	argument is OK, and indicate that it's OK for {err,warn}{,x}().

	(3) Don't let developers who do not undersand the notion that warning
	fixes MUST NOT BREAK CODE AND MUST BE CHECKED CAREFULLY AND TESTED
	make unsupervised warning-related 'fixes' to the source tree.  (This
	type of thing has happened _way_ too many times, and has wasted too
	much time and effort to be tolerated much longer.)

	Note that changing err() (or the other functions to tolerate
	this brokenness is absolutely wrong.
>Audit-Trail:
>Unformatted: