Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 20 Nov 2012 18:32:10 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Eitan Adler <eadler@FreeBSD.org>
Subject:   Re: svn commit: r243321 - head/usr.sbin/edquota
Message-ID:  <20121120180834.U1234@besplex.bde.org>
In-Reply-To: <20121120172226.R1115@besplex.bde.org>
References:  <201211200212.qAK2C1sT097754@svn.freebsd.org> <20121120172226.R1115@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 20 Nov 2012, Bruce Evans wrote:

> On Tue, 20 Nov 2012, Eitan Adler wrote:
>
>> Log:
>>  Remove unneeded includes.
>> 
>>  Tested with "make universe"; there are no conditional features.
>
> "make universe" can't find such features.  Except inversely -- when it
> doesn't find them, it usually means that there is a bug in the headers.

> [... oops; I wrote a lot about sys/stat.h, but removing it was correct]

> To remove a single header, you must know all the standard pollutions and
> check cc -E -MM output to verify that no nonstandard pollutions are
> depended on.

My unused includes checking utility doesn't do much better with the
pollution, but it finds better includes to remove: (it tends to
find pollution problems by generating large output that is not quite
so large as cc -E -MM output).  It gives:

%%%
trying /home/bde/r/edquota/edquota.c
line 55 in source /home/bde/r/edquota/edquota.c seems to be unnecessary:
#include <sys/mount.h>
line 53 in source /home/bde/r/edquota/edquota.c seems to be unnecessary:
#include <sys/stat.h>
line 52 in source /home/bde/r/edquota/edquota.c seems to be unnecessary:
#include <sys/param.h>
%%%

Removing all 3 headers doesn't work, and shows that some ufs header depends
on sys/param.h>  But removing sys/stat.h like you did and also sys/mount.h
works, and checking .depend (which is equivalent to checking cc -E -MM)
shows that neither sys/stat.h nor sys/mount.h is used.  Except I didn't
check with a universe-wide set of variations of options.

Removing the include of sys/mount.h instead of the include of sys/param.h
is correct.  In the current version, the ufs header is satisfying its
dependency on sys/param.h accidentally by getting sufficient pollution
from sys/mount.h, which it otherwise doesn't use.  Pollution in headers
makes this bug too easy to write, and dependencies on accidental pollution
make cleaning the pollution harder.

@ #!/bin/sh
@ 
@ # unusedinc - find unused `#include' lines in almost (?) any directory with
@ # a makefile.  Usage: `unusedinc' or `unusedinc file.o ...'.
@ #
@ # Known bugs:
@ # - scribbles in current directory
@ # - copying files to the current directory tends to break #include "foo.h",
@ #   e.g., in src/usr.bin/w.
@ # - doesn't handle undocumented nested includes properly.  E.g., including
@ #   <sys/time.h> to get `struct timeval' gives a free include of <time.h>,
@ #   but it is not correct to depend on this pollution.  We should run gcc -M
@ #   to separate these unused but necessary includes from the completely
@ #   unused ones.
@ # - slow.
@ 
@ # XXX removed -Wconversion since it was too painful.
@ # XXX removed -Wshadow since it was too painful.
@ # XXX removed -Wwrite-strings since it was too painful.
@ MAKEOBJDIR=/ make -n "$@" | grep '^cc .*-c ' | sed 's/cc/cc -Wall -Wbad-function-cast -Wcast-align -Wcast-qual -Wchar-subscripts -Winline -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wredundant-decls -Wstrict-prototypes -fno-builtin/' |
@ while :;
@ do
@     read line
@     if [ -z "$line" ]
@     then
@ 	rm -f 1 2 1.o 2.o z.c
@ 	exit
@     fi
@     line=`echo "$line" | sed 's; -o [^ 	]*;;'`
@     cmd=`echo "$line" | sed 's;[ 	][^ 	]*$;;'`
@     src=`echo "$line" | sed 's;.* ;;'`
@     echo "trying $src"
@     cat $src >z.c
@     $cmd z.c >/dev/null 2>1
@     if test -f z.o
@     then
@ 	mv z.o 1.o
@     else
@ 	echo "original build of $src failed!"
@ 	$cmd z.c
@ 	exit 1
@     fi
@     for i in `grep -n '^[ 	]*#include' $src | sed 's/:.*//' | sort -nr`
@     do
@ 	sed -e "$i""s/.*//" $src >z.c
@ 	$cmd z.c >/dev/null 2>2
@ 	if test -f z.o
@ 	then
@ 	    mv z.o 2.o
@ 	    if cmp -s 1 2 && cmp -s 1.o 2.o
@ 	    then
@ 		echo "line $i in source $src seems to be unnecessary:"
@ 		head -$i $src | tail -1
@ 		# exit 1
@ 	    fi
@ 	fi
@     done
@ done

unusedinc is not very good in different ways than
/usr/src/tools/tools/kerninclude/kerninclude.sh.  The latter is smarter
about annulling includes (IIRC, it builds a new include tree and
replaces 1 include at a time with an empty file, where the above removes
#include lines 1 at a time).  This gives many fewer false positives,
but makes the checking too heavyweight to use on 1 small program at a
time.  It is only implemented for the kernel, while the above works
in any reasonably- structured source directory (no subdirectories or
.PATH's pointing elsewhere), or on a single source file at a time in
such a directory.  kerninclude.sh is also unusable because its globality
and the kernel size generates more output than anyone wants to see.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121120180834.U1234>