Subject: Re: [PATCH v5] alloc: fix dangling pointer in alloc_state cleanup

From: ノウラ | Flare <[email-masked]>
Date: Fri, 5 Sep 2025 19:47:47 +0200

On 05/09/2025 16:39, Junio C Hamano wrote:
> ノウラ | Flare <[email-masked]> writes:
>
>> By *s I am referring to *s_ so a sanity check with: if (!*s_) return;
> Because we
>
> 	s = *s_;
>
> upfront, exactly because we do want the code to segfault if the
> caller passes NULL to the function (so s_ that is NULL will cause a
> NULL dereference right there), after that happens checking the NULL
> ness of s and *s_ is equivalent.
>
> And the whole point of doing "s = *s_" upfront is because readers
> can easily get confused when they have to deal with double pointers.
> The only reason why we pass the address of the pointer variable is
> so that we can assign NULL to it at the very end, and before we can
> do so, we want to be able inspect the innards of alloc_state object.
> By dereferencing s_ early into s, the code can work with the object
> itself without having to worry about following double pointer, so
> even though if (!*s_) and if (!s) may be equivalent, writing the
> latter is more in line with the whole reason why we have a variable
> 's' that is separate from 's_'.

I see your point, but I previously wrote we could lean more towards

Strict contract enforcement instead of defensive programming

Which would keep if (!s) return; instead of if (!s_ || !*s_) return;

In any case, I believe we all now agree on if (!s) return;

Pushing it right now.

Happy w-e y'all!