lkml.org 
[lkml]   [2012]   [Apr]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: CodingStyle vs checkpatch for block comments
From
Date
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.



\
 
 \ /
  Last update: 2012-04-03 19:43    [from the cache]
©2003-2011 Jasper Spaans