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

From: Fengguang Wu
Date: Mon Sep 02 2013 - 22:47:17 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/