Date: Sun, 29 Mar 2015 16:08:33 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Mark Millard <markmi@dsl-only.net> Cc: freebsd-toolchain@freebsd.org, dim@freebsd.org, brde@optusnet.com.au Subject: Re: svn commit: r280636 - head/include Message-ID: <20150329142917.P1402@besplex.bde.org> In-Reply-To: <111667CF-2A20-48CE-80F7-3F89FD274EB6@dsl-only.net> References: <111667CF-2A20-48CE-80F7-3F89FD274EB6@dsl-only.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 28 Mar 2015, Mark Millard wrote: > With reference to Dimitry Andric's Thu Mar 26 14:41:43 UTC 2015 note ... >>> [... gcc's "fixing" of headers] >> Indeed. See also this recent discussion on -current: >> >> https://lists.freebsd.org/pipermail/freebsd-current/2015-March/055111.html >> >> where a "fixed" stdio.h (from a gcc port) causes trouble. > > Preface: I do not want to be taken as indicating that GCC manipulation of headers to make alternates is a good thing: it is not. I expect that such creates lots of problems, as has been indicated. > > But I also maintain that the code sighted in https://lists.freebsd.org/pipermail/freebsd-current/2015-March/055111.html also has a problem relative to the C language standards (various vintages), independent of any gcc header manipulation of the C standard headers or FreeBSD headers. > > Setting up the context for the point: The code references the type name va_list from a #include'd header (<printf.h>) that is not from a C standard in order to declare a function that is not from a C standard (__xvprintf). <printf.h> is nonstandard, so it can do anything it wants, including requiring users of it to do certain things although that is of low quality. It does do the latter. stdio.h is fairly careful about namespace pollution, but printf.h has mounds of namespace pollution. It is not documented in any man page, and I don't like it so I try not to look at it and don't remember what it is supposed to do. Outside of libraries, it is only used in a single place in /usr/src (hastd). So it appears to be an implementation detail that escaped. It might be clean enough for an implementation detail. It has 4 namespace errors and 1 or 4 style bugs in the line that declares __xvprintf(). Almost all the identifiers have 1 or 2 bugs: X int __xvprintf(FILE *fp, const char *fmt0, va_list ap); - "int" is correct - after "int", there is the style bug not indenting the function name. All the prototypes in this file have this bug. stdio uses the very strict KNF indentation of 1 tab, then 1 space to line after the function names after putting a "*" in front of some of them - "__xvprintf" is correct - "FILE" is correct. This file has stdio.h as a prerequisite (though it includes stdio.h itself, perhaps for its broken conversion to a public header) - "fp" is broken since it is in the application namespace - "fp" is a style bug in strict KNF. Strict KNF disallows names for parameters in prototypes to avoid the previous bug in a way that I don't like. The originators of this style didn't like names with the necessary number of underscores (1). - "fmt0" and "ap" have the same 2 bugs as "fp" - "va_list" is more fundamentally broken. It is not always declared in stdio.h, so it cannot be used here. In C99, stdio.h is not permitted to declared it. I forget if C99 always leaves it in the application namespace unless <stdarg.h> is included. POSIX broke this in some cases, and FreeBSD broke it in more cases (see below). Apparently, gcc causes further problems by "fixing" some cases. Indeed, gcc48 "fixes" the broken declaration of va_list to one of __not_va_list__, and fixes the non-broken use of __va_list to use of __gnu_va_list. gcc can hard-code __gnu_va_list like this, but FreeBSD cannot. FreeBSD's __va_list is a macro that expands to __gnu_va_list iff that is correct. __not_va_list__ seems to be completely broken. Sometimes it is correct for stdio.h to declare va_list, and the "fix" breaks these cases. printf.h should take the same care as stdio.h here. For va_list, this is just to use __va_list like stdio.h does in 9 places. Most of the other bugs can be fixed by removing the parameter names. stdio.h used to be careful with parameter names, but now has bugs like missing parentheses in almost all of the uses of the parameter names in the __cplusplus section at the end of the file (these are macro parameters, so their names don't need underscores). The only obvious bugs in parameters in old code is missing parentheses for fropen() and fwopen(). The brokenness of va_list in <stdio.h> is: - POSIX.1-2001 is broken by specifying va_list to be declared in <stdio.h> for the XSI case - some time after the 2004 version on the web, but before a 2007 draft version, POSIX completed this breakage by specifying va_list to be declared in <stdio.h> unconditionally. It marks this as a C extension of course - C11 didn't break this, at least in the n1570.pdf draft. It still doesn't have it in stdio.h, and says to include both <stdarg.h> and <stdio.h> before using each of the varargs functions in <stdio.h>. Using these functions without including <stdarg.h> may even be an error if their va_list parameter is just passed to another function. FreeBSD's implementation using the __va_list spelling doesn't force an error in this case. - FreeBSD broke this for 2001 POSIX until before the version of POSIX that completed the breakage, by declaring va_list in stdio.h for all versions of POSIX >= 200112, instead of for all versions of POSIX >= 200809 or whenever it was that the bug became standard. This is in addition to declaring it for XSI like it used to be. FreeBSD never declared this under __BSD_VISIBLE like it does for the smaller XSI bugs. Declaring things for convenience in multiple headers just gives portability problems. Many headers were misdesigned, so the multiple declarations are needed for historical reasons, but stdio.h only has this bug in recent versions. > The point: That code does so before any #include <stdarg.h> happens in the #include sequence. Under the C standards one must include a standard header before one's own code can validly refer to anything that the standard header officially defines or declares. That would be correct for an application header, but since printf.h is a public header it should use __va_list and not export everything in stdarg.h. (The POSIX breakage doesn't even avoid the requirement to include stdarg.h for most uses, since not many uses need only va_list and the POSIX breakage is limited to the va_list symbol.) Since printf.h is new and unportable, it can also declare va_list unconditionally without affecting portability , but that alone is as useless as when stdio.h declares it alone. > For the C standard headers themselves (various C standard vintages): > > <stdio.h> is supposed to declare the likes of vfprintf, vprintf, and vsprintf but not the public name va_list, just using a private-named/anonymous synonym for the type to declare the functions. Also <stdio.h> is not supposed to include <stdargs.h>. The standard headers have mutual independence (so include-order-independence) and idempotent status (include-repeatability) only relative to each other, not relative to one's own code that references the standard header content. Indeed, but printf.h is not incorrect in requiring stdio.h to declare va_list. It can simply only support POSIX >= 200809 or whatever, so that the above doesn't apply to stdio.h and stdio.h is required to declare va_list. If it were documented, then its documentation could say this. gcc's "fix" is clearly broken in this case for applications too. > Where I get this from: Much of my background information for this and the terminology that I use for the C library is from The Standard C Library by P. J. Plauger, Copyright 1992. I learned a lot from that too. Also from glibc at the same time. Both are very careful about namespaces. I try to be careful about namespaces without being so ugly. One method for being not so ugly for the declaration of varargs functions in stdio.h is to hard-code the type underlying va_list instead of macroizing it. But that only works well for a single compiler on a single system for a short time. > But I've not noticed any later ANSI/ISO/IEC material indicating that the above material has changed status in more recent vintages of the standard. I could be wrong since I've not tried to be comprehensive about analyzing all the changes. I have a copy of BS ISO/IEC 9899:1999 (with Technical Corrigendum 1 incorporated and with the Revision 5.10 C Rationale) but not of the C11 vintage of the standard. Unfortunately, XSI always was broken/sloppy with almost all extensions, and POSIX standardized many old mistakes that it left out in 1988, and standards are now too large for anyone to understand (gcc is doing well to "fix" only 1 bug per large ifdef-tangled header). Details are just easier and less expensive to find on the web. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150329142917.P1402>