Re: [Ksummit-2013-discuss] [PATCH] checkpatch: Add comment about updating Documentation/CodingStyle

[Fengguang Wu <fengguang.wu@intel.com>]

On Mon, Sep 02, 2013 at 06:52:45PM -0700, Joe Perches wrote:
> On Mon, 2013-09-02 at 18:34 -0700, Josh Triplett wrote:
> > I'd suggest a couple more, which
> > *should* always make sense, and to the best of my knowledge don't tend
> > to generate false positives:
> > 
> > C99_COMMENTS
> 
> I don't have a problem with c99 comments.
> As far as I know, Linus doesn't either.
> 
> https://lkml.org/lkml/2012/4/16/473
> 
> > CONFIG_EXPERIMENTAL
> > CVS_KEYWORD
> 
> OK, but <shrug>
> 
> > ELSE_AFTER_BRACE
> 
> I wouldn't do this one.  I think
> there are some false positives here.
> 
> > GLOBAL_INITIALIZERS
> > INITIALISED_STATIC
> 
> Nor these.
> 
> > INVALID_UTF8
> > LINUX_VERSION_CODE
> > MISSING_EOF_NEWLINE
> 
> OK I suppose.
> 
> > PREFER_SEQ_PUTS
> > PRINTK_WITHOUT_KERN_LEVEL
> 
> There are a lot of these.
> I suggest no here.
> 
> > RETURN_PARENTHESES
> > SIZEOF_PARENTHESIS
> 
> It's in coding style, but some newish patches
> do avoid them.  It's a question about how noisy
> you want your robot to be.

I'd prefer the robot to show up only when necessary. The coding style
warnings are good for the developers who actively run checkpatch.pl to
make their patch better. However most are probably not suitable for a
robot to send people unsolicited warnings.

> > SPACE_BEFORE_TAB
> > TRAILING_SEMICOLON
> > TRAILING_WHITESPACE
> > USE_DEVICE_INITCALL
> 
> > USE_RELATIVE_PATH
> 
> Having checkpatch tell people how to write changelogs
> I think not a great idea.
> 
> > These *ought* to make sense, but I don't know their false positive rates:
> > 
> > HEXADECIMAL_BOOLEAN_TEST
> 
> That's a good one.  0 false positives.
> 
> > ALLOC_ARRAY_ARGS
> 
> Yes, this would be reasonable too.
> 
> > CONSIDER_KSTRTO
> 
> I think orobably not.  This would be a cleanup thing.

Perhaps we can run it for a while, so that people at least come to
aware there is a kstrto() for use. :)

> > CONST_STRUCT
> 
> OK
> 
> > SPLIT_STRING
> 
> I suggest no but <shrug>

Thanks for both of your suggestions! I'll add the commonly agreed ones:

+INVALID_UTF8
+LINUX_VERSION_CODE
+MISSING_EOF_NEWLINE
+HEXADECIMAL_BOOLEAN_TEST
+ALLOC_ARRAY_ARGS
+CONST_STRUCT
+CONSIDER_KSTRTO

And remove the duplicate one (good catch, Josh!)

-KREALLOC_ARG_REUSE

Thanks,
Fengguang