linux-kernel - Re: + checkpatch-add-check-for-use-of-sizeof-without-parenthesis.patch added to -mm tree

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: <alpine.DEB.2.00.1207091933320.25143@chino.kir.corp.google.com>
Date:	Mon, 9 Jul 2012 19:49:19 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	Joe Perches <joe@...ches.com>, linux-kernel@...r.kernel.org,
	mm-commits@...r.kernel.org, apw@...onical.com
Subject: Re: + checkpatch-add-check-for-use-of-sizeof-without-parenthesis.patch
 added to -mm tree

On Mon, 9 Jul 2012, Linus Torvalds wrote:

> sizeof without parenthesis is an abomination, and should never be used.
> 
> Sure, you don't need to have the parenthesis (except when you do - for
> actual types), but it's a parsing oddity.
> 
> The sane solution is: just add the f*cking parenthesis, and don't use
> the parsing oddity.
> 
> The parenthesis are *required* when it is a type, and they are a nice
> clarification (and makes the code easier to read) when it's an
> expression. Not having them is insane, because it just makes it clear
> how odd the parsing rules are for the two different cases.
> 

It's only strange looking because Documentation/CodingStyle doesn't use 
the gcc, glibc, C99, K&R, etc, style of doing "sizeof (struct foo)" and 
rather requires "sizeof(struct foo)".

> Think of it as a function, and get over your idiotic pissing match
> over how long you've both known C. That's irrelevant. It's a C builtin
> function with (unnecessarily) odd parsing rules that the kernel tries
> to standardize. The fact that it can take a type is the least odd part
> of it (there are other built-in C extensions that look like functions
> and do special things with the arguments they get -
> __builtin_constant_p(), __builtin_choose_expr() etc - they don't
> evaluate the *value* of the argument either).
> 

The only thing I've asked for is that something like this be added to 
Documentation/CodingStyle and not just in checkpatch.pl since hardly 
anybody uses that perl script.  You've chosen to mandate 
"sizeof(struct foo)" over "sizeof (struct foo)" in that document, which 
makes parenthesis for all unary expressions of the sizeof operator 
stylistically better as well, so it's only reasonable to ask that it 
mandates them for all unary expressions of sizeof.  It's your kernel.  
(That will be an exception for sizeof unary expressions, though, since we 
certainly don't want to mandate (i++).)

So, as I've been saying the whole thread, please either mandate it kernel-
wide in the CodingStyle or don't do it in checkpatch.  Checkpatch 
continually acts as a backdoor way of people enforcing their own 
preferences over others.
--
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/