Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
From: Joe Perches
Date: Wed Apr 05 2017 - 05:58:01 EST
On Wed, 2017-04-05 at 09:43 +0000, Hugues FRUCHET wrote:
>
> On 04/05/2017 10:35 AM, Joe Perches wrote:
> > On Wed, 2017-04-05 at 08:23 +0000, Hugues FRUCHET wrote:
> > > Hi Joe, thanks for reviewing,
> >
> > Hello Hugues
> >
> > > I have run the command you advice on the entire kernel code, modifying
> > > the script to only match the newly introduced check case.
> > > There was 14389 hits, quite huge, so I cannot 100% certify that there
> > > are no false positives, but I have checked the output carefully and
> > > found 2 limit cases:
> > >
> > > 1) space character placed just after "/*"
> > > WARNING: Block comments starts with an empty /*
> > > #330: FILE: arch/alpha/kernel/core_irongate.c:330:
> > > + /*
> > > + * Check for within the AGP aperture...
> > > => 146 hits (grep -c -n -E "\/\* $" /tmp/check.txt)
> > >
> > > 2) // style comment followed by pointer dereference
> > > WARNING: Block comments starts with an empty /*
> > > #426: FILE: drivers/media/dvb-core/dvb_ca_en50221.c:426:
> > > + // success
> > > + *tupleType = _tupleType;
> > > => 4 hits
> > >
> > > Anyway this reveal comment style related issues, so I would say that we
> > > can keep script as it is, what do you think about ?
> >
> > Glancing at the output, there is also the comment
> > in a multiline macro case:
> >
> > WARNING: Block comments starts with an empty /*
> > #354: FILE: arch/mips/include/asm/processor.h:354:
> > + /* \
> > + * Other stuff associated with the process \
> >
> > Dunno how common that is, but maybe the test
> > should be changed to avoid those.
> >
>
> Here is a proposal that remove this macro case:Per
>
> # Missing initial /*
> if ($realfile !~ m@^(drivers/net/|net/)@ && #networking exception
> $prevrawline =~ /^\+[ \t]\/\**.+[ \t]/ && #start with /*...
> $prevrawline !~ /^\+.*\/\*.*\*\/[ \t]*/ && #no inline /*...*/
> + $prevrawline !~ /^\+[ \t]\/\*+[ \t]+\\$/ &&#no macro /*<tab><\>
> $rawline =~ /^\+[ \t]*\*/ &&
> $realline > 2) {
Perhaps it's better to change this to
$prevrawline !~ /^\+\s*\/\*.*\\$/
Also perhaps the
// foo
*bar = baz;
case could be avoided by adding tests for the
comment character $; on $prevline and $line
and not looking only at $prevrawline and $rawline.