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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130903024640.GA11168@localhost>
Date:	Tue, 3 Sep 2013 10:46:40 +0800
From:	Fengguang Wu <fengguang.wu@...el.com>
To:	Joe Perches <joe@...ches.com>
Cc:	Josh Triplett <josh@...htriplett.org>,
	David Howells <dhowells@...hat.com>,
	Andy Whitcroft <apw@...onical.com>,
	ksummit-2013-discuss@...ts.linuxfoundation.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	Mauro Carvalho Chehab <m.chehab@...sung.com>,
	Wang Shilong <wangshilong1991@...il.com>
Subject: Re: [Ksummit-2013-discuss] [PATCH] checkpatch: Add comment about
 updating Documentation/CodingStyle

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/