linux-kernel - Re: [PATCH] checkpatch: fix for stripping brackets from macros.

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20171218162352.GA838@azazel.net>
Date:   Mon, 18 Dec 2017 16:23:52 +0000
From:   Jeremy Sowden <jeremy@...zel.net>
To:     Joe Perches <joe@...ches.com>
Cc:     linux-kernel@...r.kernel.org, apw@...onical.com
Subject: Re: [PATCH] checkpatch: fix for stripping brackets from macros.

On 2017-12-18, at 07:12:50 -0800, Joe Perches wrote:
> On Mon, 2017-12-18 at 14:17 +0000, Jeremy Sowden wrote:
> > When checking macros, checkpatch.pl strips parentheses, square
> > brackets and braces.  However, the search-and-replace expression was
> > not correct, and instead of replacing the brackets and their
> > contents with just the contents, it was replacing them with literal
> > 1's.
>
> Jeremy:
>
> What is the effect on the rest of the block that uses this substituted
> $dstat?  Why should this be done?

I had some macros which defined compound literals, e.g.:

  #define TEST (struct test) { .member = 1 }

and checkpatch.pl complained that macros with complex values should be
enclosed in parentheses.  When I had a look at the checkpatch.pl source
I noticed that there were a number of exceptions against which $dstat
was matched and that they included the struct and union keywords.  These
matches failed, however, 'cause the compound-literal had been turned
into:

 1 1

which didn't seem to make much sense.  Given that the while-loop was
immediately followed by another that did the more obvious thing:

  # Flatten any parentheses and braces
  while ($dstat =~ s/\([^\(\)]*\)/1/ ||
         $dstat =~ s/\{[^\{\}]*\}/1/ ||
         $dstat =~ s/.\[[^\[\]]*\]/1/)
  {
  }

  # Flatten any obvious string concatentation.
  while ($dstat =~ s/($String)\s*$Ident/$1/ ||
         $dstat =~ s/$Ident\s*($String)/$1/)
  {
  }

it occurred to me that this might be a bug.

> Andy:
>
> I believe this is intentional as it simplifies
> the macro analysis and has no other effect on the
> rest of the block.  Correct?
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -4874,9 +4874,9 @@ sub process {
> >  			$dstat =~ s/\s*$//s;
> >
> >  			# Flatten any parentheses and braces
> > -			while ($dstat =~ s/\([^\(\)]*\)/1/ ||
> > -			       $dstat =~ s/\{[^\{\}]*\}/1/ ||
> > -			       $dstat =~ s/.\[[^\[\]]*\]/1/)
> > +			while ($dstat =~ s/\(([^\(\)]*)\)/$1/ ||
> > +			       $dstat =~ s/\{([^\{\}]*)\}/$1/ ||
> > +			       $dstat =~ s/.\[([^\[\]]*)\]/$1/)
> >  			{
> >  			}

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)