> -----Original Message-----
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Maxim
> Uvarov
> Sent: Monday, May 22, 2017 10:07 PM
> To: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCHv2] scripts: checkpatch: update to allow
> additional exceptions
> 
> Merged!
> 
> Maxim.
> 
> On 05/04/17 22:33, Bill Fischofer wrote:
> > Update checkpatch.pl to avoid issuing warnings for use of externs,
> > volatile, or camelCase.
> >
> > Signed-off-by: Bill Fischofer <bill.fischo...@linaro.org>
> > ---
> >  scripts/checkpatch.pl | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 16316b92..1c27ac60 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -4273,7 +4273,7 @@ sub process {
> >
>       $camelcase_file_seeded = 1;
> >                                             }
> >                                     }
> > -                                   if (!defined
> $camelcase{$word}) {
> > +                                   if (!defined
> $camelcase{$word} && 0) {


First, I think it's not good to edit the checkpatch.pl itself. We should just 
use the config file to document what checks are ignored. Also these direct 
edits are lost when we upgrade to new checkpatch version.

Also, I think camel case check is useful. We are forced to use camel case 
sometimes due to external lib (openSSL) API, but those are exceptions and 
should be handled as such. Now this edit opens door for every patch to contain 
camel case, also when there's no reason to do so. We need reviewers to check 
for it now, which is a waste.

So, I'd suggest to revert this.



> >
>       $camelcase{$word} = 1;
> >
>       CHK("CAMELCASE",
> >
> "Avoid CamelCase: <$word>\n" . $herecurr);
> > @@ -4620,7 +4620,7 @@ sub process {
> >
> >  # no volatiles please
> >             my $asm_volatile =
> qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
> > -           if ($line =~ /\bvolatile\b/ && $line !~
> /$asm_volatile/) {
> > +           if ($line =~ /\bvolatile\b/ && 0 && $line !~
> /$asm_volatile/) {
> >                     WARN("VOLATILE",
> >                          "Use of volatile is usually wrong:


Why not just add --ignore=VOLATILE into checkpatch.conf ?



> see Documentation/volatile-considered-harmful.txt\n" . $herecurr);
> >             }
> > @@ -5134,7 +5134,7 @@ sub process {
> >                     if (defined $cond) {
> >                             substr($s, 0, length($cond),
> '');
> >                     }
> > -                   if ($s =~ /^\s*;/ &&
> > +                   if ($s =~ /^\s*;/ && 0 &&
> >                         $function_name ne 'uninitialized_var')
> >                     {
> >                             WARN("AVOID_EXTERNS",
> >

Why not just add --ignore= AVOID_EXTERNS into checkpatch.conf ?


It seems that the entire commit should be reverted and config file edited 
instead.

-Petri





Reply via email to