Re: [PATCH 9/9] CodingStyle: flip the rule about curlies

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 5/13/2025 12:06 PM, Alexey Dobriyan wrote:
> On Mon, May 12, 2025 at 06:56:56PM +0200, Greg KH wrote:
>> On Mon, May 12, 2025 at 09:43:10AM -0700, Jeff Johnson wrote:
>>> On 5/9/2025 11:18 PM, Greg KH wrote:
>>>> On Fri, May 09, 2025 at 11:34:30PM +0300, Alexey Dobriyan wrote:
>>>>> Require set of curlies {} in all if/else branches and all loops
>>>>> not matter how simple.
>>>>
>>>> Sorry, but no, we are not going to change this long-term coding style
>>>> rule for no real reason at this point in time.
>>>
>>> Is the infamous Apple SSL bug (CVE-2014-1266) a good reason?
> 
> Indeed.
> 
> Thanks, curlies were inspired by this CVE but I forgot to mention it.
> 
>> One bug in 2014 will require us to touch 30+ million lines of code?
> 
> Nobody is proposing to reformat 30 mil lines at one commit
> (as much as I'd like it).
> 
> Old code will stay old, new code will be formatted per new rules.
> 
>> Please be reasonable.
> 
> I'm very reasonable. Each patch details rationale why specific style is
> better.
> 
>> And everyone, remember _why_ we have a coding style.  It's not so much
>> the specifics of _what_ the coding style is,
> 
> What? When was the last time you read it? It is very much about specifics:
> 8 spaces, opening curly on the same line except at function scope,
> 80 columns, recent rule about to format function attributes.
> 
> It could have even more specific if there is pre-commit hook forcing
> formatting like commercial companies do.
> 
>> one at all.  Don't argue the specifics of the coding style without a
>> really really good reason why, with real details and proof.
> 
> What is "really really good"?
> 
> How do you know when it is good reason or not?
> 
> I think I have good reason: I programmed a little in another languages
> where some of the rules don't apply. In particular C++/Rust don't have
> a rule about declaring variables upfront. Nor does any popular programming
> language designed in the last 35 years (?).
> 
> Such experience made me realize that linux-kernel CodingStyle in this
> regard is pointless at best and counter-productive. It was so obvious.
> 
>> It took us a long time to increase the default line length, and that too
>> is still argued about for very good and valid reasons.
> 
> It still 80 columns in CodingStyle.
> 
>> That was discussed in detail, not just thrown at us like this patch series was.
> 
> Oh come on. In Russia we say "not my first year of marriage".
> 
> One of the unwritten rules of linux-kernel is to NEVER post [RFC]
> as it will be ignored, but to post a [PATCH] and Cc specific people
> to force a discussion.
> 
> I don't want to look like a thief who sneaks in occasional declaration
> in the middle of a function or set of curlies and get yelled by compilers
> or maintainers (especially those armed with checkpatch.pl).
> 
> I'll codify this first in CodingStyle, then delete relevant checks from
> checkpatch.pl (citing CodingStyle of course).

I replied only this patch in the series because, when I first started
programming using the Linux Coding Style in 2004, the single statement brace
rule was the only rule I disagreed with. And that was in part due to the fact
that, at three of my previous employers, the C coding style had dictated
mandatory use of braces. So the explicit prohibition of braces for single
statement conditionals really surprised me.

And note the rule, as written, is not what actually seems to be enforced (at
least by checkpatch.pl). What seems to be enforced is to not use braces where
the conditional is a single line and there is a single line statement. In
other words, if either the conditional or the single statement span multiple
lines, then braces are allowed (or encouraged?). As examples, checkpatch.pl
does not complain about any of the following (either to recommend adding
braces or removing braces):

	if (a_really_long_line_function_name(a_really_long_identifier) ||
	    another_really_long_function_name(another_long_identifier)) {
		braced();
	}

	if (a_really_long_line_function_name(a_really_long_identifier) ||
	    another_really_long_function_name(another_long_identifier))
		not_braced();

	if (a)
		call_a_really_long_function(with_long_argument,
					    another_log_argument);

	if (a) {
		call_a_really_long_function(with_long_argument,
					    another_log_argument);
	}

I'll also note that the popular style guide published by Michael Barr as well
as the MISRA C standard also dictate braces (but I'll admit that both of these
have other rules which contradict the Linux style and where I agree with the
Linux style).

All of that said, ultimately, the Coding Style is supposed to be enforce
consistent style that is readable and maintainable. I get that. I personally
believe that requiring braces makes the code more maintainable, and doesn't
detract from readability. However I'm also agree that changing the rule would
need to eventually lead to changing all the code that doesn't conform (since
one of the reasons for having a style is to have consistency), so I'm also
swayed by admonition to not "argue the specifics of the coding style without a
really really good reason why, with real details and proof," so I'll let
others have their say.

/jeff