On Fri, Sep 05, 2025 at 02:02:39AM +0200, ノウラ | Flare wrote:
> > + if (!s) return;
>
> However, I agree with Peff. After calling alloc_state_free_and_null(&foo)
> Having foo == NULL is an expected behavior, especially since the function
> Is designed to free the memory and null out the caller’s pointer using
> A double pointer ensuring the helper is idempotent
>
> So, calling it again on the same pointer is safe because it simply
> no-ops if the memory is already freed
>
> Regarding the sanity check, it should be:
>
> + if (!*s) return;
With the correction in your follow-up that this should be:
if (!*s_) return;
I agree that is the right thing. But it is equivalent to:
if (!s) return;
since we'll have just assigned "s". Which one to choose is purely a
matter of style. Using "*s_" perhaps makes it more clear that we are
sanity-checking the input (and could happen even before we assign "s").
Using "s" is consistent with the rest of the function in working with
the more direct pointer value. I am happy with either.
> The reasoning is the following:
>
> => s is the double pointer (the address of the caller’s pointer)
> => *s is the actual pointer to the memory we want to free
>
> Thus, we check !*s to allow the function to safely handle already-NULL
> pointers
> But if we instead checked !s then we would be testing whether the caller
> passed
> A NULL double pointer which is a programmer error
> So silently returning on !s would hide a bug
I think this is wrong. "s" is the value we would have allocated from
alloc_state_new(), and what we are actually interested in freeing. We
only see the double-pointer "s_" because we want to modify the caller's
containing pointer.
The FREE_AND_NULL() macro does not itself have this same confusion
because it _is_ a macro. So it is expanded in the caller's context and
does not need the extra layer of indirection.
I do not think this is a good idea, because it obscures things further
for people who are accustomed to C idioms, but one could imagine a more
general macro like:
#define FREE_AND_NULL_WITH(p, fn) do { fn(p); (p) = NULL; } while (0)
#define FREE_AND_NULL(p) FREE_AND_NULL_WITH(p, free)
and then you could do:
FREE_AND_NULL_WITH(o->blob_state, alloc_state_free);
where alloc_state_free() would only receive the single pointer "s", and
would free it but not assign NULL.
-Peff
