Eric Blake <ebl...@redhat.com> writes:
> On 01/31/2018 08:48 AM, Markus Armbruster wrote:
>> System headers should be included with <...>, our own headers with
>> "...". Offenders tracked down with an ugly, brittle and probably
>> buggy Perl script. Previous iteration was commit a9c94277f0.
>>
>> Put the cleaned up system header includes first, except for the ones
>> the next commit will delete.
>>
>> While there, separate #include from file comment with a blank line,
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>
>> +++ b/target/i386/hvf/x86_mmu.c
>> @@ -15,18 +15,17 @@
>> * You should have received a copy of the GNU Lesser General Public
>> * License along with this program; if not, see
>> <http://www.gnu.org/licenses/>.
>> */
>> +
>> #include "qemu/osdep.h"
>> +#include <memory.h>
>
> <memory.h> is an obsolete spelling for the now-universal <string.h>. We
> should NEVER need to include it; scripts/clean-includes should be taught
> to blacklist this one.
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the Universe trying to
produce bigger and better idiots. So far, the Universe is winning."
My point is: if we extended our tooling every time we see a mistake,
we'll drown in tooling. I think we should limit ourselves to *common*
mistakes. Is this one common?
> But that's a matter for another patch; this one does exactly what it
> promises, and was mechanical (even if the brittle perl script isn't
> published), so it's best to keep it as-is.
I append my Perl script.
Its simpleminded approach to locating headers in the source tree fails
frequently. It should really use the compiler to find them.
I reviewed inclusions of headers the script flags as "(included
inconsistently)". I also reviewed "(included with <>)" where the script
can find a header in the tree. I patched only obvious mistakes. There
are a few unobvious cases: headers in tree with the same name as a
system header, e.g. elf.h, io.h.
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
#!/usr/bin/perl -w
use strict;
my %ihdr = ();
my %idir = ();
my @sall = ();
my @sinc = ();
open(my $fh, "-|", "git grep '^[ \t]*#[ \t]*include[ \t]'")
or die "can't run git grep: $!";
while (<$fh>) {
m,^(([^:]*)/)?[^:/]*:[ \t]*\#[ \t]*include[ \t]*(["<])([^">]*), or next;
my $dir = $2 // ".";
my $delim = $3;
my $h = $4;
$ihdr{$h} |= 1 << ($delim eq "<");
if (exists $idir{$h}) {
my $aref = $idir{$h};
push @$aref, $dir unless grep($_ eq $dir, @$aref);
} else {
$idir{$h} = [$dir];
}
}
close ($fh);
open($fh, "-|", "git ls-tree -r --name-only HEAD")
or die "can't run git ls-tree: $!";
while (<$fh>) {
chomp;
push @sall, $_;
}
close ($fh);
@sinc = grep(/^include\//, @sall);
sub pr {
my ($h, $fn, $src) = @_;
print "$h -> $fn";
if ($ihdr{$h} == 3) {
print " (included inconsistently)";
} elsif ($src) {
print " (included with <>)" if ($ihdr{$h} != 1);
} else {
print " (included with \"\")" if ($ihdr{$h} != 2);
}
print "\n";
}
for my $h (keys %ihdr) {
$h =~ m,^(\.\./)*(include/)?(.*), or die;
my $hh = $3;
my @fn = grep(/^include\/\Q$hh\E$/, @sinc);
if (@fn) {
pr($h, $fn[0], 1);
next;
}
@fn = grep(/^\Q$hh\E$/, @sall);
if (@fn) {
pr($h, $fn[0], 1);
next;
}
for my $dir (@{$idir{$h}}) {
next if $dir eq ".";
print "## $dir/$hh\n";
@fn = grep(/^\Q$dir\/$hh\E$/, @sall);
if (@fn) {
pr($h, $fn[0], 1);
} else {
pr($h, "? (in $dir)", 0);
}
}
}
