Re: CodingStyle question: multiple statements on a single line

From: Mike Isely
Date: Tue Sep 02 2008 - 19:58:26 EST


On Tue, 2 Sep 2008, Roland Dreier wrote:

> > 2) No, never use the 'if (a) b;' construction. Put 'b;' on the next line
> > instead.
>
> This is correct. Always write simple if statements as
>
> if (a)
> b;

Going back to the text:

<quote>
Don't put multiple statements on a single line unless you have
something to hide:
</quote>

Then what does "unless you have something to hide" refer to exactly?


<quote>
if (condition) do_this;
do_something_everytime;
</quote>

Realize that "if (condition) do_this;" is in fact one statement. The
"if (condition)" part is not something that can stand on its own; it is
a predicate which is not completed without the rest of the statement.
I interpret the example to be showing what is correct, not what is
disallowed. Both lines are single statements, each on their own line.
I see the above as trying to illustrate outlawing of this sort of
silliness:

if (condition) do_this; do_something_everytime;

which will compile and run but is obviously misleading.

If anything the "do_this" is an example where one has something to hide,
since it is not in the normal flow of execution (predicated instead by
"if (condition)").

If anything,

if (condition) do_this;

is safer than

if (conditon)
do_this;

because while in both cases it's one statement, the second case is split
in exactly the spot where somebody's later errant line of code (say, a
debug printk) will completely change what is going on. With the whole
statement on one line, this situation is avoided. Or perhaps

if (condition) {
do_this;
}

is another way to avoid the same issue, but that seems frowned upon for
other reasons (below).

I know the answer to all this is just "but nobody does it that way".
But my reading of the CodingStyle says this is allowed and that is what
I thought was being asked - what does CodingStyle say?


>
> > And in general, why is this:
> >
> > if (a) {
> > b;
> > }
> >
> > not accepted by the CodingStyle? (At least as I understand it)
>
> The braces take up another line of whitespace, which means less code
> fits on the screen. And in simple cases, they don't add anything.
> Finally, the vast majority of the kernel leaves the braces off, so they
> look funny to people who read a lot of kernel code.

So by that reasoning "if (a) b;" - provided it stays under 80 columns -
should be even better. It occupies less space so that more code can fit
on the screen.


>
> And uniformity counts for a lot: most coding style rules are completely
> arbitrary, but having a uniform kernel style makes reading kernel code
> much easier.

What about drivers? The statement has been made by others that there is
a strong desire for outside drivers to be brought into mainline rather
than being out-of-tree. So must every chunk of code brought in this way
be sanitized to this level of detail? In many cases that can be a large
(and some might say arbitrary) hurdle.


>
> Keep in mind that common sense always trumps any mechanical rule. So if
> there is some case where writing
>
> if (a) {
> b;
> }
>
> is clearly easier to read than leaving the braces off, then that would
> be OK.

That's great. How does one reconcile this statement with subsystem
maintainers who treat checkpatch.pl - which is the epitome of
"mechanical rule" and has no notion of common sense - as the gatekeeper
for all incoming changesets?

-Mike


--

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
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/