Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200623103740.25876-1-quentin@isovalent.com> <895052df95b2fc85b07e5a493fbb1ff415d8f62c.camel@perches.com>
In-Reply-To: <895052df95b2fc85b07e5a493fbb1ff415d8f62c.camel@perches.com>
From:   Quentin Monnet <quentin@isovalent.com>
Date:   Tue, 23 Jun 2020 23:15:32 +0100
Message-ID: <CACdoK4KknQMerx0k=bZxyQ35v1geGw99CitpP4hHMZ_o7W2eiA@mail.gmail.com>
Subject: Re: [PATCH v2] checkpatch: fix CONST_STRUCT when const_structs.checkpatch
 is missing
To:     Joe Perches <joe@perches.com>
Cc:     Andy Whitcroft <apw@canonical.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 23 Jun 2020 at 17:48, Joe Perches <joe@perches.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 a=
n
> > external file (scripts/const_structs.checkpatch), in commit bf1fa1dae68=
e
> > ("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 .=3D '|' if ($$wordsRef ne "");
> > +                     $$wordsRef .=3D '|' 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-undef=
ined-variable-in-perl-with-warnings-enabled

I didn't want to touch $typeOtherTypedefs in v2, and I thought I
needed that =C2=AB $$wordsRef ne "" =C2=BB 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