From: ノウラ | Flare Date: Fri, 05 Sep 2025 17:47:47 GMT Subject: Re: [PATCH v5] alloc: fix dangling pointer in alloc_state cleanup Message-ID: <44ae54a6-ba49-40e7-b519-15c12cca0404@gmail.com> In-Reply-To: On 05/09/2025 16:39, Junio C Hamano wrote: > ノウラ | Flare 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!