ksummit.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>,
	ksummit@lists.linux.dev, Dan Williams <dan.j.williams@intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Dan Carpenter <dan.carpenter@linaro.org>
Subject: Re: Clarifying confusion of our variable placement rules caused by cleanup.h
Date: Wed, 31 Dec 2025 13:17:32 +0100	[thread overview]
Message-ID: <aVUUXAKjiNroU5tR@black.igk.intel.com> (raw)
In-Reply-To: <7b37e1cb-271e-49fe-a3ee-5443006284e1@p183>

On Tue, Nov 25, 2025 at 05:25:19PM +0300, Alexey Dobriyan wrote:
> On Tue, Nov 18, 2025 at 11:39:26AM -0500, James Bottomley wrote:
> 
> > So which should we do?
> 
> The best way to understand that C89 style of declaring in the beginning
> of the function is pointless rule is to write some code in a language
> which doesn't enforce it. You should see that nothing bad happens.
> 
> It increases bug rate due to increased variable scope allowing typos.
> 
> It bloats LOC -- in many cases declaration and initializer can fit
> into a single line.

It's a weak argument, see below.

> It prevents adding "const" qualifier if necessary.
> 
> Pressing PageUp and PageDown when adding new variable is pointless
> busywork and distracts, breaks the tempo(flow?) so to speak.

I think with something like VSCode, it's much easier to handle, but it's just a
side note.

> C89 style provokes substyles(!) which makes adding new variables even
> more obnoxious: some subsystems have(had?) a rule saying that declarations
> (with initializers) must be sorted by length, so not only programmer has
> to PageUp to the beginning of the block, but then aim carefully and
> insert new declaration.
> 
> None of this is necessary (or possible) if the rule says "declare as low
> as possible".

This is a bad rule, if hard defined, as it provokes to have a code like

	int ret = foo(...);

	if (ret)
		do_smth(...);

The evolution of such code is prone to subtle mistakes, as more code will be
added during the development of the mentioned function. I saw the real error
case when somebody does something wrong in between the lines. That's why I
prefer the assignment to be split from the definition.

	int ret;

	ret = foo(...);
	if (ret)
		do_smth(...);

is more maintainable and robust.

However, if we talk about RAII variables, the assignment and definition makes
a lot of sense to go together.

> There was variation of this type of nonsense with headers (not only it has
> to be sorted alphabetically but by length too!)

By length it indeed sounds weird, but alphabetical is the natural language
order everybody learnt from the daycare / school years, so it's properly
programmed in our deep brain. Having that allows to find easily if anything one
is interested in is already being included. Also it allows to avoid dup inclusions
(was there, fixed that for real). So, it's not bad.

> There is no practical difference between code and declarations:
> declarations can have initializers which can be arbitrary complex,
> just like "real" code. So the only difference is superficial.
> 
> C89 declaration style is pointless and dumb, no wonder other programming
> languages dumped it (or never had), it should be simply discarded.
> 
> It will also make Linux slightly less white crow to newcomers
> (C++ doesn't have this rule after all).

-- 
With Best Regards,
Andy Shevchenko



      parent reply	other threads:[~2025-12-31 12:17 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-18 16:39 James Bottomley
2025-11-18 17:18 ` Bart Van Assche
2025-11-18 18:38   ` Linus Torvalds
2025-11-18 19:04     ` Bart Van Assche
2025-11-18 19:14       ` Linus Torvalds
2025-11-18 20:43         ` Al Viro
2025-11-18 19:15       ` H. Peter Anvin
2025-11-18 19:11     ` H. Peter Anvin
2025-11-18 19:16       ` Linus Torvalds
2025-11-18 19:19         ` H. Peter Anvin
2025-11-18 19:17     ` Steven Rostedt
2025-11-18 19:22       ` Linus Torvalds
2025-11-18 19:56         ` Steven Rostedt
2025-11-18 20:23           ` Linus Torvalds
2025-11-18 21:05             ` Linus Torvalds
2025-11-18 20:21       ` James Bottomley
2025-11-18 20:30         ` Linus Torvalds
2025-11-18 20:51         ` Steven Rostedt
2025-11-18 21:10           ` James Bottomley
2025-11-18 22:34             ` Steven Rostedt
2025-11-18 23:32               ` James Bottomley
2025-11-18 19:23 ` H. Peter Anvin
2025-11-18 20:28   ` James Bottomley
2025-11-25 13:09 ` Jani Nikula
2025-11-25 14:25 ` Alexey Dobriyan
2025-11-25 15:32   ` Stephen Hemminger
2025-11-25 16:04   ` Steven Rostedt
2025-11-25 17:57   ` H. Peter Anvin
2025-12-31 12:17   ` Andy Shevchenko [this message]

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=aVUUXAKjiNroU5tR@black.igk.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=adobriyan@gmail.com \
    --cc=dan.carpenter@linaro.org \
    --cc=dan.j.williams@intel.com \
    --cc=ksummit@lists.linux.dev \
    --cc=linux-kernel@vger.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