[Libvir] [PATCH] * qemud/remote.c: Don't include <getopt.h>. Not used.
I had a few in-progress changes from a week or two ago, and am clearing the decks. I added a new build-checking rule (coming separately) and it exposed an unnecessary include: * qemud/remote.c: Don't include <getopt.h>. Not used. diff --git a/qemud/remote.c b/qemud/remote.c index 85df9cd..c7db5fa 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -41,7 +41,6 @@ #include <syslog.h> #include <string.h> #include <errno.h> -#include <getopt.h> #include <ctype.h> #include <fnmatch.h> -- 1.5.4.4.482.g16f99
On Wed, Mar 19, 2008 at 02:08:44PM +0100, Jim Meyering wrote:
I had a few in-progress changes from a week or two ago, and am clearing the decks.
I added a new build-checking rule (coming separately) and it exposed an unnecessary include:
+1 So we have a way to find header files which are unused? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
"Richard W.M. Jones" <rjones@redhat.com> wrote:
On Wed, Mar 19, 2008 at 02:08:44PM +0100, Jim Meyering wrote:
I had a few in-progress changes from a week or two ago, and am clearing the decks.
I added a new build-checking rule (coming separately) and it exposed an unnecessary include:
+1
So we have a way to find header files which are unused?
For some simple ones, yes. See the rules like sc_prohibit_assert_without_use in the existing Makefile.maint. Yesterday I factored out the core of those four rules and now have the following. (a patch to update this is coming up) The hard part is coming up with a regular expression to indicate whether a particular header is used. The following are simple-minded, but so far they seem to do the job... However, for a header with many functions and macros, the required regexp will be a lot more complicated. # To use this "command" macro, you must first define two shell variables: # h: the header, enclosed in <> or "" # re: a regular expression that matches IFF something provided by $h is used. define _header_without_use h_esc=`echo "$$h"|sed 's/\./\\./'`; \ if $(CVS_LIST_EXCEPT) | grep '\.c$$' > /dev/null; then \ files=$$(grep -l '^# *include '"$$h_esc" \ $$($(CVS_LIST_EXCEPT) | grep '\.c$$')) && \ grep -LE "$$re" $$files | grep . && \ { echo "$(ME): the above files include $$h but don't use it" \ 1>&2; exit 1; } || :; \ else :; \ fi endef # Prohibit the inclusion of assert.h without an actual use of assert. sc_prohibit_assert_without_use: @h='<assert.h>' re='\<assert *\(' $(_header_without_use) # Prohibit the inclusion of getopt.h without an actual use. sc_prohibit_getopt_without_use: @h='<getopt.h>' re='\<getopt(_long)? *\(' $(_header_without_use) # Don't include quotearg.h unless you use one of its functions. sc_prohibit_quotearg_without_use: @h='"quotearg.h"' re='\<quotearg(_[^ ]+)? *\(' $(_header_without_use) # Don't include quote.h unless you use one of its functions. sc_prohibit_quote_without_use: @h='"quote.h"' re='\<quote(_n)? *\(' $(_header_without_use)
On Wed, Mar 19, 2008 at 02:21:26PM +0100, Jim Meyering wrote:
The hard part is coming up with a regular expression to indicate whether a particular header is used. The following are simple-minded, but so far they seem to do the job... However, for a header with many functions and macros, the required regexp will be a lot more complicated.
Hmmm ... sounds like a job for CIL ... http://et.redhat.com/~rjones/cil-analysis-of-libvirt/ Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Wed, Mar 19, 2008 at 01:11:18PM +0000, Richard W.M. Jones wrote:
On Wed, Mar 19, 2008 at 02:08:44PM +0100, Jim Meyering wrote:
I had a few in-progress changes from a week or two ago, and am clearing the decks.
I added a new build-checking rule (coming separately) and it exposed an unnecessary include:
+1
So we have a way to find header files which are unused?
No - this is impossible unless you have a copy of every OS we've ever tested on. It may be unused on Linux, but may be needed on Solaris, etc etc. Removing <getopt.h> is an obviously safe action, but in general we should be wary of removing supposedly unused heads. Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, Mar 19, 2008 at 01:25:33PM +0000, Daniel P. Berrange wrote:
On Wed, Mar 19, 2008 at 01:11:18PM +0000, Richard W.M. Jones wrote:
On Wed, Mar 19, 2008 at 02:08:44PM +0100, Jim Meyering wrote:
I had a few in-progress changes from a week or two ago, and am clearing the decks.
I added a new build-checking rule (coming separately) and it exposed an unnecessary include:
+1
So we have a way to find header files which are unused?
No - this is impossible unless you have a copy of every OS we've ever tested on. It may be unused on Linux, but may be needed on Solaris, etc etc. Removing <getopt.h> is an obviously safe action, but in general we should be wary of removing supposedly unused heads.
Surely we can do it for POSIX calls? Of course the OS / libc itself may not obey POSIX ... Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Wed, Mar 19, 2008 at 01:50:27PM +0000, Richard W.M. Jones wrote:
On Wed, Mar 19, 2008 at 01:25:33PM +0000, Daniel P. Berrange wrote:
On Wed, Mar 19, 2008 at 01:11:18PM +0000, Richard W.M. Jones wrote:
On Wed, Mar 19, 2008 at 02:08:44PM +0100, Jim Meyering wrote:
I had a few in-progress changes from a week or two ago, and am clearing the decks.
I added a new build-checking rule (coming separately) and it exposed an unnecessary include:
+1
So we have a way to find header files which are unused?
No - this is impossible unless you have a copy of every OS we've ever tested on. It may be unused on Linux, but may be needed on Solaris, etc etc. Removing <getopt.h> is an obviously safe action, but in general we should be wary of removing supposedly unused heads.
Surely we can do it for POSIX calls?
Of course the OS / libc itself may not obey POSIX ...
Yeah, I'm not convinced any OS is fully compliant with POSIX header file definitions - particularly when you get into more obscure platforms like win32/cygwin, or even just slightly older Linux. And we're compiling with the _USE_GNU extension defined so the headers we're including on Linux aren't even in POSIX compliant mode anyway Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Wed, Mar 19, 2008 at 01:50:27PM +0000, Richard W.M. Jones wrote:
On Wed, Mar 19, 2008 at 01:25:33PM +0000, Daniel P. Berrange wrote:
On Wed, Mar 19, 2008 at 01:11:18PM +0000, Richard W.M. Jones wrote:
On Wed, Mar 19, 2008 at 02:08:44PM +0100, Jim Meyering wrote:
I had a few in-progress changes from a week or two ago, and am clearing the decks.
I added a new build-checking rule (coming separately) and it exposed an unnecessary include:
+1
So we have a way to find header files which are unused?
No - this is impossible unless you have a copy of every OS we've ever tested on. It may be unused on Linux, but may be needed on Solaris, etc etc. Removing <getopt.h> is an obviously safe action, but in general we should be wary of removing supposedly unused heads.
Surely we can do it for POSIX calls?
Of course the OS / libc itself may not obey POSIX ...
Yeah, I'm not convinced any OS is fully compliant with POSIX header file definitions - particularly when you get into more obscure platforms like win32/cygwin, or even just slightly older Linux. And we're compiling with the _USE_GNU extension defined so the headers we're including on Linux aren't even in POSIX compliant mode anyway
It is obviously feasible to detect at least some unused headers. Just be very careful before removing any #include directive that a new check claims is unused. Obviously there's a cost/gain trade-off. E.g. my existing getopt check is sloppy in that it doesn't bother checking for optind or opterr, or any of the other symbols typically defined in getopt.h. It just checks for the regexp '\<getopt(_long)? *(', since so far that has been sufficient.
Automated builds on a variety of platforms would help here ... Tricky for us to set up however. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
participants (3)
-
Daniel P. Berrange -
Jim Meyering -
Richard W.M. Jones