Messages in this thread |  | | | Subject | Re: CodingStyle vs checkpatch for block comments | | From | Joe Perches <> | | Date | Tue, 03 Apr 2012 10:41:47 -0700 |
| |
On Tue, 2012-04-03 at 13:25 -0400, Chris Metcalf wrote: > (Adding linux-kernel to the cc's and restating the question...) > > I accepted a patch from Kautuk Consul and adjusted his comment style, but > he pointed out that he had changed his comment style based on a warning
It's a check not a warning.
> from checkpatch. > > The Documentation/CodingStyle file says: > > The preferred style for long (multi-line) comments is: > > /* > * This is the preferred style for multi-line > * comments in the Linux kernel source code. > * Please use it consistently. > * > * Description: A column of asterisks on the left side, > * with beginning and ending almost-blank lines. > */ > > However, a new change to "checkpatch --strict" by Joe Perches, commit > aad4f61498, causes this construct to be flagged as a warning
checkpatch flags it with a "check" only when --strict is on the checkpatch command line.
> if (and only > if) it is preceded by a blank line. Joe said the change was to support > David Miller's preferred style, but that he didn't much care one way or > another.
I think the block comment style is less important than the actual content of the block comment. The actual comment content is rather difficult for checkpatch to parse though.
> The relevant code in checkpatch.pl is: > > if ($rawline =~ /^\+[ \t]*\/\*[ \t]*$/ && > $prevrawline =~ /^\+[ \t]*$/) { > CHK("BLOCK_COMMENT_STYLE", > "Don't begin block comments with only a /* > line, use /* comment...\n" . $hereprev); > } > > So, my questions - > > 1. I'm not sure what the regexps are really trying to avoid. Presumably a > blank line followed by a block comment is OK? Certainly the kernel sources > are full of this construct.
It emits a check message on <blank line> /* but not <blank line> /* some actual comment > 2. The actual warning message emitted seems to directly contradict the > CodingStyle document, so presumably we should either clarify the message, > or update CodingStyle if we're really trying to change the style.
Or just remove it or add a test for the patched file to be in net/... or drivers/net... or something.
|  |