Message-ID: <58fd478f408a34b578ee8d949c5c4b4da4d4f41d.camel@HansenPartnership.com>
Date: Tue, 18 Nov 2025 11:39:26 -0500
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: ksummit@...ts.linux.dev, Dan Williams <dan.j.williams@...el.com>
Cc: linux-kernel <linux-kernel@...r.kernel.org>, Dan Carpenter
	 <dan.carpenter@...aro.org>
Subject: Clarifying confusion of our variable placement rules caused by
 cleanup.h

This patch, introduced by Dan Williams (cc'd):

https://lore.kernel.org/all/171097196970.1011049.9726486429680041876.stgit@dwillia2-xfh.jf.intel.com/

There was some discussion at the time (I missed it because I'm not on
any of the lists it touched) and people expected to be opinionated were
cc'd but none of it really touched on the problem that we now have
inconsistent rules for variable and initializer placement.

Before this change, we always adhered to K&R rules, subsequently
codified in C89, to place variable declarations at the top of the block

The problem specifically is this added comment in cleanup.h:

>  * That bug is fixed by changing init() to call guard() and define +
>  * initialize @obj in this order::
>  *
>  *	guard(mutex)(&lock);
>  *	struct object *obj __free(remove_free) = alloc_add();

Which is recommending mixing declarations and code contrary to our
prior rule.  I note the rule against mixing variables and code was
relaxed in the C99 standard (and in a lot of other languages), but
we've never formally changed our coding rules.

I'm not saying we have to stick with C89, just that if we change
adherence to it, we should do so globally and document it because
having incosistency for __free vs other variables really isn't a good
idea.

So which should we do?  It does seem the dependency ordering problem of
__free annotations means that there are situations where we can't
adhere to the variables at the top of the block rule, but are we going
to relax this globally or simply advise noting the dependency in a
comment but keep the rule (thus discouraging the mixing of code and
declarations except where absolutely necessary)?

The other problem is that we've traditionally not initialized automatic
variables specifically to avoid the initialization taking unnecessary
space in the data/bss of the kernel.  However immediately below the
quote above, there's this

>  *
>  * Given that the "__free(...) = NULL" pattern for variables defined at
>  * the top of the function poses this potential interdependency problem
>  * the recommendation is to always define and assign variables in one
>  * statement and not group variable definitions at the top of the
>  * function when __free() is used.

Which advises always initializing __free variables (in addition to
mixing code and declarations).  My reading of our compilers and
checkers is that actually this is unnecessary: there is a danger that a
return before initialization will result in a free being called on an
uninitialized pointer, but that the checkers and compilers can
correctly detect this (at least as correctly as they usually detect
uninitialized variables) so, again, requiring __free variables always
to be initiallised is an unnecessary deviation from our usual coding
habits.

The danger of the above is that one way of avoiding the data/bss space
problem is to move the declaration into the code directly where the
allocation is (as the rule relaxation above says you could).  Again,
I'm not saying we shouldn't do this, just that we should do it
consistently and mindfully rather than allowing it to leak in like
this.

So, again, I think we should debate whether and how much we're changing
the coding rules and do so globally in the coding-style document rather
than keeping the change in the cleanup.h file.

For myself I do find some value in the C89 declarations at the
beginning of the block for readability, so I'm happy to relax the
mixing rule to cases where it's strictly necessary and require
documenting in the comment what the necessity is.  However, I do think
we should, absent ordering problems, keep __free variables
uninitialised and at the top of the block given we can detect any
problem (and thus keep this rule absolutely for non-__free variables
where there's no ordering issues).  But, again, I'm less attached to
this position than I am to the consistency one: I really think it's a
bad idea to change the rules for one class of variables but not for
another, so whatever we do, we should do it for everything and if that
means relaxing the rule mixing code and declarations for everthing, I
can live with that.

Regards,

James

Powered by blists - more mailing lists