linux-kernel - Re: [PATCH] vmalloc: remove #ifdef in function body

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] [day] [month] [year] [list]

Message-ID: <20111221152101.GD24863@thunk.org>
Date:	Wed, 21 Dec 2011 10:21:01 -0500
From:	Ted Ts'o <tytso@....edu>
To:	Michal Nazarewicz <mina86@...a86.com>
Cc:	Joe Perches <joe@...ches.com>, Minchan Kim <minchan@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-mm <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] vmalloc: remove #ifdef in function body

On Wed, Dec 21, 2011 at 07:47:17AM +0100, Michal Nazarewicz wrote:
> This patch that you pointed to is against “#ifdefs are ugly” style
> described in Documentation/SubmittingPatches.
> 
> >If it's not in coding style, I suggest
> >it should be changed if it doesn't
> >add some other useful value.
> 
> That my be true.  I guess no one took time to adding it to the document.

Like all things, judgement is required.  Some of the greatest artists
know when it's OK (and in fact, a good thing) to break the rules.
Beethoven, for example, broke the rules when he added a chorus of
singers to his 9th Symphony.  Take a look at Bach's chorales,
universally acknowledged to be works of genius.  Yet there Bach has
occasionally double thirds, crossed voices, and engaged in parallel
fifths --- and big no-no's which go against the textbook rules of
chorale writing.

In this case, if you have an #ifdef surrounding an entire function
body, I think common sense says that the it's fine.  There's also the
rule which is says that all other things being equal, it's better not
to waste vertical space and useless boiler plate.

Worst of all is patches to change perfectly existing code because
someone is trying to be a stickler for rules, since it can break other
people's patches.  If you are need to make a change, it's best that it
be checkpatch clean.  But sending random cleanups just because someone
is trying to get their patch count in the kernel higher is Just
Stupid, and should be strongly discouraged.

(And that last, too, is a rule that has exceptions...)

							- Ted
--
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/