On Tue, May 23, 2017 at 7:07 AM, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolai...@nokia.com> wrote:
> > > > -----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. > > The camel case rule we want is very simple: you may use camel case symbols but you may not define any new ones. I don't believe checkpatch is set up to support that rule, however. Is there a way of doing this to your knowledge? > > > > > > > $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. > That's a good suggestion. > > -Petri > > > > > >
