Re: [PATCH v2] check headers for complete includes, etc.

[Jörn Engel]

On Wed, 1 October 2003 18:39:30 +0200, Sam Ravnborg wrote:
> On Wed, Oct 01, 2003 at 11:48:25AM +0200, Jörn Engel wrote:
> > 
> > If there are no big complaints, I consider this version to be final.
> Some small comments..
> 
> > --- /dev/null	1970-01-01 01:00:00.000000000 +0100
> > +++ linux-2.6.0-test5/scripts/checkheader.pl	2003-09-30 21:41:14.000000000 +0200
> > @@ -0,0 +1,54 @@
> > +#!/usr/bin/perl -w
> > +use strict;
> > +
> > +my $verbose = 1;	# TODO make this optional
> Could you make this controlable with option -verbose, see below.

ok.

> > +my @headers = sort(map({prune($_);}
> > +	`(cd include/ && find linux -name "*.h" ; find asm/ -name "*.h")`));
> 1) you uses include but asm/ - use final '/' for consistency.
              ^^^^^^^
You meant linux, right?  Agreed.

> 2) Using asm/ you require the symlink to be present. Which obvious
> it a most when  doing this check, so we better secure that.

ok.

> > +my $basename = "lib/header";
> I much rather have it be: include/headercheck
> then people realise where eventual temporary files comes from.

ok.

> So we need something like the following here: (untested)
> >  
> > +headercheck: prepare-all
> > +	$(PERL) scripts/checkheader.pl $(if $(KBUILD_VERBOSE),-verbose)
> > +
> >  versioncheck:
> >  	find * $(RCS_FIND_IGNORE) \
> >  		-name '*.[hcS]' -type f -print | sort \
> 
> With respect to passing on to Linus.
> I would like to have a bunch of files converted first so we can see the
> effect of requiring this check.
> Also I am still a bit uncertain if this is the right approach,
> but until now this is the only patch...

I share your doubts.  Considering that the header files only rarely
cause problems, some hundred hits sound a bit too much.  Also, this
should be extended to also warn on unnecessary headers being included,
so it drives us both ways.  But then again, this is not important
enough to me right now to spend too much time on it.

Jörn

-- 
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/