Date: Sun, 13 Dec 2009 04:48:45 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Xin LI <delphij@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r200420 - in head/usr.bin: ar c89 calendar cksum cmp colcrt colrm compress cpuset expand fetch file2c find finger fmt fold gcore getopt hexdump jot killall ktrace lastcomm limits lock l... Message-ID: <20091213031047.X32430@delplex.bde.org> In-Reply-To: <200912112335.nBBNZdRT023731@svn.freebsd.org> References: <200912112335.nBBNZdRT023731@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 11 Dec 2009, Xin LI wrote:
> Log:
> Remove unneeded header includes from usr.bin/ except contributed code.
Surely many of these are used?
> Tested with: make universe
Removing includes cannot be tested in this way, since it doesn't provide
clean includes, and FreeBSD includes are extremely polluted, so upon
removal of some apparently-unused includes, things will compile
accidentally due to the pollution.
> Modified:
> head/usr.bin/ar/util.c
> head/usr.bin/c89/c89.c
> head/usr.bin/calendar/calendar.c
> head/usr.bin/calendar/day.c
> head/usr.bin/calendar/ostern.c
> head/usr.bin/calendar/paskha.c
> head/usr.bin/cksum/Makefile
> head/usr.bin/cksum/crc.c
> head/usr.bin/cksum/crc32.c
> head/usr.bin/cksum/sum1.c
> head/usr.bin/cksum/sum2.c
> ...
For example, most source files in cksum use *intN_t, and these types
are only guaranteed (by both C99 and POSIX) to be declared in <stdint.h>
(and by Standard namespace pollution, also in <inttypes.h>, and for a
couple of these types, also in places like <netinit.in.h>, so most
source files in cksum obviously need to include <stdint.h> like they
used to. For these types, the pollution in FreeBSD is mainly in
<sys/types.h>, where many of these types are declared under an XXX
comment about this being a bug, together with much larger pollution
involving <sys/types> itself being included a lot, so this and other
pollution in it affect many other headers. Removing the used includes
of <stdint.h> makes it even harder to clean up <sys/types.h> :-(.
> Modified: head/usr.bin/cksum/crc.c
> ==============================================================================
> --- head/usr.bin/cksum/crc.c Fri Dec 11 23:30:22 2009 (r200419)
> +++ head/usr.bin/cksum/crc.c Fri Dec 11 23:35:38 2009 (r200420)
> @@ -44,7 +44,6 @@ __FBSDID("$FreeBSD$");
>
> #include <sys/types.h>
My unusedinc utilty also reports that most of the includes of <sys/types.h>
"seem to be unused", but this is wrong too. Here <sys/types.h> is a
still documented prerequisite for many (probably most) of the functions
in <unistd.h> (and thus for any include of <unistd.h>), but this is
another bug...
>
> -#include <stdint.h>
My unusedinc utilty used to report that this "seems to be unused". It is
no smarter than "make universe" here. This utility can also be run on
kernels, where it reports tens of thousands of "seems to be unused", each
of which needs to be examined individually but this takes too long so
I stopped doing it about 10 years ago. /usr/src/tools/tools/kerninclude/
kerninclude.sh uses a more robust algorithm for determining unusedinc'ness,
but it doesn't work in userland. (To test the unusedness of <sys/stdint.h>,
it replaces <sys/stdint.h> by an empty file. This removes nested pollution
of symbols defined in <sys/stdint.h> but not direct pollution of the form
#ifndef _UINT32_T_DECLARED
typedef __uint32_t uint32_t;
#define _UINT32_T_DECLARED
#endif
, if any. Note that in some cases a direct declaration of *intN_t is
required by POSIX, so it is not always pollution, unlike for all the
direct declarations of this form in <sys/types.h>.)
> #include <unistd.h>
<unistd.h> includes <sys/types.h> next to an XXX comment about this being
a bug. (POSIX allows this low quality but IIRC doesn't require it, and
it certainly doesn't require <unistd.h> to inherit bugs from <sys/types.h>.
(It is allowed to declare any foo_t, but it is not permitted to define
most of the other pollution that FreeBSD's <sys/types.h> defines (mainly
endianness and select stuff IIRC).))
Related bugs:
- in POSIX.1-1988 through POSIX.1-1996, <sys/types.h> was required before
most POSIX headers including <unistd.h>, and many (probably most)
section 2 man pages in FreeBSD still document this requirement.
- However, <unistd.h> has had the polluting include of <sys/types.h> in
it since rev.1.1 (just without the XXX in rev.1.1), so this requirement
never applied to any of the functions in <unistd.h>. I fixed a lot of
section to man pages to require <sys/types.h> iff it was actually required
by FreeBSD headers, and intentionally didn't change the ones for functions
in <unistd.h> since the changes would have to be undone when FreeBSD catches
up with POSIX.1-2000, but some imports from OtherBSD didn't follow this
policy so we may have more man pages documenting the old-POSIX requirements
than we started with.
>
> #include "extern.h"
This uses *intN_t, so it depends on <stdint.h> or pollution.
cksum also has a few bogus uses of u_char, u_int and u_long. The uses
of u_char and u_long seem to be just bugs (I think the u_char's need
to be uint8_t, and the u_int for lcrc needs to be at least uint17_t
(elsewhere we are more careful and use uintN_t for all CRCs). IIRC,
POSIX requires unsigned char uint8_t and u_int to be at least uint32_t,
so these bugs are harmless on POSIX systems). The uses of u_long are
only to print uint32_t's so they are only portabilty bugs.
cksum still spells strrchar as rindex.
Apart from these bugs, cksum seems to be a POSIX.1-2008 application: it
compilese cleanly with -D_POSIX_C_SOURCE=200809 -Du_char=int8_t
-Du_int=uint32_t -Du_long="unsigned long" -Drindex=strrchr.
> Modified: head/usr.bin/cksum/crc32.c
> ==============================================================================
> --- head/usr.bin/cksum/crc32.c Fri Dec 11 23:30:22 2009 (r200419)
> +++ head/usr.bin/cksum/crc32.c Fri Dec 11 23:35:38 2009 (r200420)
> @@ -17,7 +17,6 @@ __FBSDID("$FreeBSD$");
> #include <sys/types.h>
>
> #include <stdio.h>
> -#include <stdint.h>
> #include <unistd.h>
>
> #include "extern.h"
Similarly for all other files in cksum. I didn't check any other
directories (without more automation it can take hours per #include
to untangle/understand the pollution so as to verify removals, except
for cases that you already understand).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20091213031047.X32430>