Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Fri, 2017-04-07 at 09:56 +, Hugues FRUCHET wrote: > Hi Joe, Hi again Hugues. > here is the output with the last version of the script: > https://paste.ubuntu.com/24333124/ > > Differences are on the macro cases and the //foo \ *bar, no more warned. Thanks. I guess my only real concern about this test is there are ~15000 instances of this in the tree. Do maintainers care if comments are formatted /* * [multiple...] * line comment */ vs /* [multiple...] * line comment */ enough to want others to submit patches changing from the latter style? The reason the networking checking exists is because David Miller, the primary networking maintainer, was constantly telling others to resubmit patches to his preferred style. I doubt there's another maintainer that cares that much one way or another. I don't. Any opinions from anyone else?
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Fri, 2017-04-07 at 09:56 +, Hugues FRUCHET wrote: > Hi Joe, Hi again Hugues. > here is the output with the last version of the script: > https://paste.ubuntu.com/24333124/ > > Differences are on the macro cases and the //foo \ *bar, no more warned. Thanks. I guess my only real concern about this test is there are ~15000 instances of this in the tree. Do maintainers care if comments are formatted /* * [multiple...] * line comment */ vs /* [multiple...] * line comment */ enough to want others to submit patches changing from the latter style? The reason the networking checking exists is because David Miller, the primary networking maintainer, was constantly telling others to resubmit patches to his preferred style. I doubt there's another maintainer that cares that much one way or another. I don't. Any opinions from anyone else?
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
Hi Joe, here is the output with the last version of the script: https://paste.ubuntu.com/24333124/ Differences are on the macro cases and the //foo \ *bar, no more warned. BR, Hugues. On 04/05/2017 03:26 PM, Hugues Fruchet wrote: > > > On 04/05/2017 11:55 AM, Joe Perches wrote: >> On Wed, 2017-04-05 at 09:43 +, Hugues FRUCHET wrote: >>> >>> On 04/05/2017 10:35 AM, Joe Perches wrote: On Wed, 2017-04-05 at 08:23 +, 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]+\\$/ &
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
Hi Joe, here is the output with the last version of the script: https://paste.ubuntu.com/24333124/ Differences are on the macro cases and the //foo \ *bar, no more warned. BR, Hugues. On 04/05/2017 03:26 PM, Hugues Fruchet wrote: > > > On 04/05/2017 11:55 AM, Joe Perches wrote: >> On Wed, 2017-04-05 at 09:43 +, Hugues FRUCHET wrote: >>> >>> On 04/05/2017 10:35 AM, Joe Perches wrote: On Wed, 2017-04-05 at 08:23 +, 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]+\\$/
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On 04/05/2017 11:55 AM, Joe Perches wrote: > On Wed, 2017-04-05 at 09:43 +, Hugues FRUCHET wrote: >> >> On 04/05/2017 10:35 AM, Joe Perches wrote: >>> On Wed, 2017-04-05 at 08:23 +, 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]+\\$/ &
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On 04/05/2017 11:55 AM, Joe Perches wrote: > On Wed, 2017-04-05 at 09:43 +, Hugues FRUCHET wrote: >> >> On 04/05/2017 10:35 AM, Joe Perches wrote: >>> On Wed, 2017-04-05 at 08:23 +, 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]+\\$/
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Wed, 2017-04-05 at 09:43 +, Hugues FRUCHET wrote: > > On 04/05/2017 10:35 AM, Joe Perches wrote: > > On Wed, 2017-04-05 at 08:23 +, 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]+\\$/ &
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Wed, 2017-04-05 at 09:43 +, Hugues FRUCHET wrote: > > On 04/05/2017 10:35 AM, Joe Perches wrote: > > On Wed, 2017-04-05 at 08:23 +, 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]+\\$/
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On 04/05/2017 10:35 AM, Joe Perches wrote: > On Wed, 2017-04-05 at 08:23 +, 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: # Missing initial /* if ($realfile !~ m@^(drivers/net/|net/)@ && #networking exception $prevrawline =~ /^\+[ \t]\/\**.+[ \t]/ && #start with /*... $prevrawline !~ /^\+.*\/\*.*\*\/[ \t]*/ && #no inline /*...*/ + $prevrawline !~ /^\+[ \t]\/\*+[ \t]+\\$/ &
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On 04/05/2017 10:35 AM, Joe Perches wrote: > On Wed, 2017-04-05 at 08:23 +, 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: # Missing initial /* if ($realfile !~ m@^(drivers/net/|net/)@ && #networking exception $prevrawline =~ /^\+[ \t]\/\**.+[ \t]/ && #start with /*... $prevrawline !~ /^\+.*\/\*.*\*\/[ \t]*/ && #no inline /*...*/ + $prevrawline !~ /^\+[ \t]\/\*+[ \t]+\\$/
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Wed, 2017-04-05 at 08:23 +, 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.
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Wed, 2017-04-05 at 08:23 +, 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.
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
Hi Joe, thanks for reviewing, 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 ? Here is the count detail by first level of directories within kernel: $ grep FILE /tmp/check.txt | grep -c "FILE: drivers/" 8859 $ grep FILE /tmp/check.txt | grep -c "FILE: arch/" 2306 $ grep FILE /tmp/check.txt | grep -c "FILE: fs/" 1136 $ grep FILE /tmp/check.txt | grep -c "FILE: sound/" 810 $ grep FILE /tmp/check.txt | grep -c "FILE: include/" 669 $ grep FILE /tmp/check.txt | grep -c "FILE: kernel/" 143 $ grep FILE /tmp/check.txt | grep -c "FILE: security/" 112 $ grep FILE /tmp/check.txt | grep -c "FILE: lib/" 91 $ grep FILE /tmp/check.txt | grep -c "FILE: tools/" 81 $ grep FILE /tmp/check.txt | grep -c "FILE: crypto/" 54 $ grep FILE /tmp/check.txt | grep -c "FILE: scripts/" 44 $ grep FILE /tmp/check.txt | grep -c "FILE: mm/" 35 $ grep FILE /tmp/check.txt | grep -c "FILE: block/" 27 $ grep FILE /tmp/check.txt | grep -c "FILE: virt/" 8 $ grep FILE /tmp/check.txt | grep -c "FILE: samples/" 5 $ grep FILE /tmp/check.txt | grep -c "FILE: ipc/" 5 $ grep FILE /tmp/check.txt | grep -c "FILE: certs/" 1 The complete output is there for reference: http://paste.ubuntu.com/24319042/ Best regards, Hugues. On 04/03/2017 09:06 PM, Joe Perches wrote: > On Mon, 2017-04-03 at 10:08 +0200, Hugues Fruchet wrote: >> Warn when block comments are not starting with blank comment: >> >> /* multiple lines >> * block comment, >> * => warning >> */ >> >> /* >> * multiple lines >> * block comment, >> * => no warning >> */ >> >> Exception made for networking files where rule is the >> exact opposite. > > I recall there was some reason I didn't do this > when adding the block comment code, but I don't > recall what it was. Perhaps it was the initial > line of files. > > Maybe your $realline > 2 test fixes it. Maybe not. > Dunno. > > If you run this against the entire kernel code > using a unique test type and not BLOCK_COMMENT_STYLE > are there any false positives? > > Maybe test with something like: > > $ git ls-files -- "*.[ch]" | \ > xargs --max-args 20 ./scripts/checkpatch.pl -f --types= >
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
Hi Joe, thanks for reviewing, 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 ? Here is the count detail by first level of directories within kernel: $ grep FILE /tmp/check.txt | grep -c "FILE: drivers/" 8859 $ grep FILE /tmp/check.txt | grep -c "FILE: arch/" 2306 $ grep FILE /tmp/check.txt | grep -c "FILE: fs/" 1136 $ grep FILE /tmp/check.txt | grep -c "FILE: sound/" 810 $ grep FILE /tmp/check.txt | grep -c "FILE: include/" 669 $ grep FILE /tmp/check.txt | grep -c "FILE: kernel/" 143 $ grep FILE /tmp/check.txt | grep -c "FILE: security/" 112 $ grep FILE /tmp/check.txt | grep -c "FILE: lib/" 91 $ grep FILE /tmp/check.txt | grep -c "FILE: tools/" 81 $ grep FILE /tmp/check.txt | grep -c "FILE: crypto/" 54 $ grep FILE /tmp/check.txt | grep -c "FILE: scripts/" 44 $ grep FILE /tmp/check.txt | grep -c "FILE: mm/" 35 $ grep FILE /tmp/check.txt | grep -c "FILE: block/" 27 $ grep FILE /tmp/check.txt | grep -c "FILE: virt/" 8 $ grep FILE /tmp/check.txt | grep -c "FILE: samples/" 5 $ grep FILE /tmp/check.txt | grep -c "FILE: ipc/" 5 $ grep FILE /tmp/check.txt | grep -c "FILE: certs/" 1 The complete output is there for reference: http://paste.ubuntu.com/24319042/ Best regards, Hugues. On 04/03/2017 09:06 PM, Joe Perches wrote: > On Mon, 2017-04-03 at 10:08 +0200, Hugues Fruchet wrote: >> Warn when block comments are not starting with blank comment: >> >> /* multiple lines >> * block comment, >> * => warning >> */ >> >> /* >> * multiple lines >> * block comment, >> * => no warning >> */ >> >> Exception made for networking files where rule is the >> exact opposite. > > I recall there was some reason I didn't do this > when adding the block comment code, but I don't > recall what it was. Perhaps it was the initial > line of files. > > Maybe your $realline > 2 test fixes it. Maybe not. > Dunno. > > If you run this against the entire kernel code > using a unique test type and not BLOCK_COMMENT_STYLE > are there any false positives? > > Maybe test with something like: > > $ git ls-files -- "*.[ch]" | \ > xargs --max-args 20 ./scripts/checkpatch.pl -f --types= >
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Mon, 2017-04-03 at 10:08 +0200, Hugues Fruchet wrote: > Warn when block comments are not starting with blank comment: > > /* multiple lines > * block comment, > * => warning > */ > > /* > * multiple lines > * block comment, > * => no warning > */ > > Exception made for networking files where rule is the > exact opposite. I recall there was some reason I didn't do this when adding the block comment code, but I don't recall what it was. Perhaps it was the initial line of files. Maybe your $realline > 2 test fixes it. Maybe not. Dunno. If you run this against the entire kernel code using a unique test type and not BLOCK_COMMENT_STYLE are there any false positives? Maybe test with something like: $ git ls-files -- "*.[ch]" | \ xargs --max-args 20 ./scripts/checkpatch.pl -f --types=
Re: [PATCH v1] checkpatch: test missing initial blank line in block comment
On Mon, 2017-04-03 at 10:08 +0200, Hugues Fruchet wrote: > Warn when block comments are not starting with blank comment: > > /* multiple lines > * block comment, > * => warning > */ > > /* > * multiple lines > * block comment, > * => no warning > */ > > Exception made for networking files where rule is the > exact opposite. I recall there was some reason I didn't do this when adding the block comment code, but I don't recall what it was. Perhaps it was the initial line of files. Maybe your $realline > 2 test fixes it. Maybe not. Dunno. If you run this against the entire kernel code using a unique test type and not BLOCK_COMMENT_STYLE are there any false positives? Maybe test with something like: $ git ls-files -- "*.[ch]" | \ xargs --max-args 20 ./scripts/checkpatch.pl -f --types=
