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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 28, 2025, 20:01 UTC
Message-ID: <xmqqo6rzusex.fsf@gitster.g>
In-Reply-To: <xmqqtt1rut1w.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 8 quoted lines

>> Style nit, we tend to use the "imperative form" here in Git,
>> like this:
>>
>> - Rename allocate_alloc_state() → alloc_state_alloc().
>> - Replace ...
>> - Update ...
>
> Thanks.  We also tend to avoid bulleted list.

Perhaps it needs a bit of clarification.

If you truly require a bulleted list to enumerate what you did in the patch, then you may be doing too many things in a single patch.

But in this case, the changes do not even have to be broken down into bullets. If you change a called function, it goes without saying that you have to adjust existing callers. Perhaps something like

    All callers of clear_alloc_state() immediately free what they
    cleared, so currently it does not hurt anybody that the
    alloc_state is left in an unreusable state, but it is an
    error-prone API.  Replace it with a new function that clears but
    in addition frees the structure, as well as NULLing the pointer
    that points at it and adjust existing callers.

    While at it, rename allocate_alloc_state() and name the new
    function alloc_state_free_and_null(), to follow more closely the
    function naming convention specified in the CodingGuidelines
    (namely, functions about S are named with S_ prefix and then
    verb).

should be more in line with what we usually do.

Thanks.