linux-kernel - Re: [RFC PATCH] checkpatch.pl: Add warning on non #define continuation lines

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 2 Feb 2010 12:06:23 -0500
From:	Mike Frysinger <vapier.adi@...il.com>
To:	John Kacur <jkacur@...hat.com>
Cc:	Joe Perches <joe@...ches.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Andy Whitcroft <apw@...onical.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] checkpatch.pl: Add warning on non #define 
	continuation lines

On Tue, Feb 2, 2010 at 08:49, John Kacur wrote:
> On Tue, Feb 2, 2010 at 1:55 PM, Joe Perches wrote:
>> On Tue, 2010-02-02 at 11:08 +0000, Mark Brown wrote:
>>> it'd be good to also check for just regular use of
>>> continuations in code other than macro definitions.  These are just a
>>> style nit but if there's a script that filters out false positives from
>>> the macros that'd be handy...
>>
>>> Running "grep ' \\$' sound/soc/blackfin/*.[ch]" suggests that there's
>>> still some of the continuations I mentioned above in there (plus a lot
>>> of false positives from macros).
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 3257d3d..a174501 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1234,6 +1234,7 @@ sub process {
>>
>>        $realcnt = 0;
>>        $linenr = 0;
>> +       my $in_define = 0;
>>        foreach my $line (@lines) {
>>                $linenr++;
>>
>> @@ -1388,6 +1389,16 @@ sub process {
>>                        WARN("adding a line without newline at end of file\n" . $herecurr);
>>                }
>>
>> +               if ($rawline =~ /\\$/) {
>> +                   if ($rawline =~ /\s*\#\s*define\s+/) {
>> +                       $in_define = 1;
>> +                   } elsif (!$in_define) {
>> +                       WARN("Continuations outside of macros should be avoided\n" . $herecurr);
>> +                   }
>> +               } else {
>> +                   $in_define = 0;
>> +               }
>> +
>>  # Blackfin: use hi/lo macros
>>                if ($realfile =~ m@...h/blackfin/.*\.S$@) {
>>                        if ($line =~ /\.[lL][[:space:]]*=.*&[[:space:]]*0x[fF][fF][fF][fF]/) {
>>
>>find ./ -name "*.[ch]" | xargs ./scripts/checkpatch.pl -f | grep -a3 Continuations
>
> Checkpatch is already kind of loud, so I'm not sure I like the idea -
> you even say yourself that it's just a style nit.
> But just in-case there are a lot of people who do like this, then you
> have to do some work to get rid of false positives. Try something like
> the following.
>
> 1. apply your patch.
> 2. run something like
> find ./ -name "*.[ch]" | xargs ./scripts/checkpatch.pl -f | grep -a3
> Continuations
>
> and then go through the results looking for false positives.
>
> One that I see right away, is with #if
> For example,
>
> #39: FILE: arch/arm/mach-lh7a40x/lcd-panel.h:39:
> +#if defined (MACH_LPD79520)\
>
> WARNING: Continuations outside of macros should be avoided
> #40: FILE: arch/arm/mach-lh7a40x/lcd-panel.h:40:
> + || defined (MACH_LPD79524)\

those examples are missing spaces before the \ ;)
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/