linux-kernel - Re: [PATCH v2] checkpatch: fix CONST_STRUCT when const

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: <CACdoK4KknQMerx0k=bZxyQ35v1geGw99CitpP4hHMZ_o7W2eiA@mail.gmail.com>
Date:   Tue, 23 Jun 2020 23:15:32 +0100
From:   Quentin Monnet <quentin@...valent.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Andy Whitcroft <apw@...onical.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] checkpatch: fix CONST_STRUCT when const_structs.checkpatch
 is missing

On Tue, 23 Jun 2020 at 17:48, Joe Perches <joe@...ches.com> wrote:
>
> Hi again.
>
> On Tue, 2020-06-23 at 11:37 +0100, Quentin Monnet wrote:
> > Checkpatch reports warnings when some specific structs are not declared
> > as const in the code. The list of structs to consider was initially
> > defined in the checkpatch.pl script itself, but it was later moved to an
> > external file (scripts/const_structs.checkpatch), in commit bf1fa1dae68e
> > ("checkpatch: externalize the structs that should be const"). This
> > introduced two minor issues:
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -770,7 +770,7 @@ sub read_words {
> >                               next;
> >                       }
> >
> > -                     $$wordsRef .= '|' if ($$wordsRef ne "");
> > +                     $$wordsRef .= '|' if (defined($$wordsRef) && $$wordsRef ne "");
>
> perl is a weird language and the $$wordsRef ne "" test
> isn't required as the append will work even if the
> thing being appended to isn't defined.
>
> You can read the perlsyn docs
> http://perldoc.perl.org/perlsyn.html
> or
> https://stackoverflow.com/questions/2166575/when-is-it-ok-to-use-an-undefined-variable-in-perl-with-warnings-enabled

I didn't want to touch $typeOtherTypedefs in v2, and I thought I
needed that « $$wordsRef ne "" » for the
"read_words(\$typeOtherTypedefs, $typedefsfile)" where the variable
was defined but possibly empty? Anyway, thanks for the lore and the
links :).

>
> so perhaps remove the test and improve the additional
> $typedefsfile use too

Agreed, your version looks better. I'll resubmit a v3 with your
suggestions, please feel free to comment and add a "Co-authored-by:"
tag if you feel this is appropriate.

Thank you,
Quentin