From: Ted Ts'o <tytso@mit.edu>
To: Michal Nazarewicz <mina86@mina86.com>
Cc: Joe Perches <joe@perches.com>, Minchan Kim <minchan@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
linux-mm <linux-mm@kvack.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] vmalloc: remove #ifdef in function body
Date: Wed, 21 Dec 2011 10:21:01 -0500 [thread overview]
Message-ID: <20111221152101.GD24863@thunk.org> (raw)
In-Reply-To: <op.v6tug3vi3l0zgt@mpn-glaptop>
On Wed, Dec 21, 2011 at 07:47:17AM +0100, Michal Nazarewicz wrote:
> This patch that you pointed to is against a??#ifdefs are uglya?? 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-12-21 15:21 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-21 5:17 Minchan Kim
2011-12-21 5:31 ` Joe Perches
2011-12-21 5:45 ` Minchan Kim
2011-12-21 5:58 ` Joe Perches
2011-12-21 6:07 ` Minchan Kim
2011-12-21 6:27 ` Joe Perches
2011-12-21 6:21 ` Michal Nazarewicz
2011-12-21 6:32 ` Joe Perches
2011-12-21 6:47 ` Michal Nazarewicz
2011-12-21 15:21 ` Ted Ts'o [this message]
2011-12-21 6:13 ` Michal Nazarewicz
2011-12-21 6:22 ` Minchan Kim
2011-12-21 6:24 ` Michal Nazarewicz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111221152101.GD24863@thunk.org \
--to=tytso@mit.edu \
--cc=akpm@linux-foundation.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mina86@mina86.com \
--cc=minchan@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox