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 14:49:25 +0100
From:	John Kacur <jkacur@...hat.com>
To:	Joe Perches <joe@...ches.com>
Cc:	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Andy Whitcroft <apw@...onical.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mike Frysinger <vapier.adi@...il.com>,
	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 1:55 PM, Joe Perches <joe@...ches.com> 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)\
--
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/