From: Dan Carpenter <dan.carpenter@linaro.org>
To: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Ally Heev <allyheev@gmail.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scsi: fix uninitialized pointers with free attr
Date: Tue, 18 Nov 2025 17:22:27 +0300 [thread overview]
Message-ID: <aRyBI_j77mQaQxgT@stanley.mountain> (raw)
In-Reply-To: <9c62ff497aa00bcdf213f579272d3decdd22ea34.camel@HansenPartnership.com>
On Tue, Nov 18, 2025 at 08:21:16AM -0500, James Bottomley wrote:
> On Tue, 2025-11-18 at 09:17 +0300, Dan Carpenter wrote:
> > On Thu, Nov 06, 2025 at 11:06:29AM -0500, James Bottomley wrote:
> [...]
> > > However, why would we treat a __free variable any differently from
> > > one without the annotation? The only difference is that a function
> > > gets called on it before exit, but as long as something can detect
> > > calling this on uninitialized variables their properties are
> > > definitely no different from non-__free variables so the can be
> > > treated exactly the same.
> > >
> > > To revisit why we do this for non-__free variables: most people
> > > (although there are definitely languages where this isn't true and
> > > people who think we should follow this) think that having variables
> > > at the top of a function (or at least top of a code block) make the
> > > code easier to understand. Additionally, keeping the variable
> > > uninitialized allows the compiler to detect any use before set
> > > scenarios, which can be somewhat helpful detecting code faults (I'm
> > > less persuaded by this, particularly given the number of false
> > > positive warnings we've seen that force us to add annotations,
> > > although this seems to be getting better).
> > >
> > > So either we throw out the above for everything ... which I really
> > > wouldn't want, or we enforce it for *all* variables.
> > >
> >
> > Yeah. You make a good point...
> >
> > On the other hand, a bunch of maintainers are convinced that every
> > free variable should be initialized to a valid value at declaration
> > time and will reject patches which don't do that.
>
> Which maintainers?
Here is an example where Krzysztof says "This is not recommended way of
using cleanup. You should declare it with constructor."
https://lore.kernel.org/all/a2fc02e2-d75a-43ed-8057-9b3860873ebb@kernel.org/
I know there are other people who feel this way as well but I can't
recall who. It's in the documentation in include/linux/cleanup.h
* 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.
regards,
dan carpenter
next prev parent reply other threads:[~2025-11-18 14:22 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-05 14:14 [PATCH] scsi: fix uninitialized pointers with free attr Ally Heev
2025-11-05 14:21 ` James Bottomley
2025-11-05 14:46 ` Dan Carpenter
2025-11-05 15:32 ` James Bottomley
2025-11-06 14:46 ` Dan Carpenter
2025-11-06 16:06 ` James Bottomley
2025-11-18 6:17 ` Dan Carpenter
2025-11-18 13:21 ` James Bottomley
2025-11-18 14:22 ` Dan Carpenter [this message]
2025-11-19 6:56 ` ally heev
2025-11-19 8:31 ` Christoph Hellwig
2025-11-19 12:43 ` James Bottomley
2025-11-20 4:15 ` Martin K. Petersen
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=aRyBI_j77mQaQxgT@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=James.Bottomley@hansenpartnership.com \
--cc=allyheev@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.