Re: [PATCH] scsi: fix uninitialized pointers with free attr

[James Bottomley <James.Bottomley@HansenPartnership.com>]

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?  The true evil I see here is rule inconsistency
because it leads to confusion and bad coding.  So I'd hope if's fairly
easy to point out the errors ... and if they want to argue for
consistently coding everything with either variables always initialized
or variables declared in code, I'm sure they'll get their day.

> I see checkpatch as a way of avoiding this round trip where a patch
> is automatically rejected because of something trivial.

But you aren't just doing that ... what you're actually doing is
forcing bad coding rules on the rest of us.  Why else would I see a
patch in SCSI trying to move something that's correctly coded according
to our usual variable rules to this?

> The truth is that the cleanup.h stuff is really new and I don't think
> we've necessarily figured out all the best practices yet.

I get that ... and I'm not saying we shouldn't change stuff because of
it, I'm just saying that any rule we do change should be reasonable and
consistently applied.

Regards,

James