From: Jacob Keller <jacob.e.keller@intel.com>
Date: Thu, 26 Feb 2026 23:33:48 GMT
Subject: Re: [PATCH v7 1/4] help: use list_aliases() for alias listing
Message-ID: <71a2c638-9dc4-4add-9306-b4110e6c499f@intel.com>
In-Reply-To: <xmqq8qcga6ya.fsf@gitster.g>



On 2/25/2026 12:45 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Jacob Keller <jacob.e.keller@intel.com> writes:
>>
>>> SUMMARY: AddressSanitizer: 1453 byte(s) leaked in 37 allocation(s).
>>>
>>> This leak occurs because you now copy and store the value of the alias
>>> in the util element, but the call of list_aliases() in list_cmd()
>>> doesn't clean these up, since its string_list_clear passes 0 to the
>>> free_util argument.
>>>
>>> The following fixed it for me:
>>>
>>> diff --git c/git.c i/git.c
>>> index 744cb6527e06..aeb099ab1162 100644
>>> --- c/git.c
>>> +++ i/git.c
>>> @@ -119,7 +119,7 @@ static int list_cmds(const char *spec)
>>>           }
>>>           for (size_t i = 0; i < list.nr; i++)
>>>                   puts(list.items[i].string);
>>> -       string_list_clear(&list, 0);
>>> +       string_list_clear(&list, 1);
>>>           return 0;
>>>    }
>>
>> Thanks. This looks like one of the right things to do.  I checked
>> all list_*() that are called from the loop in this list_cmds(), and
>> list_aliases() is the only thing that uses .util member of the
>> string_list_item instances.
>>
>> However, we need to be a bit careful with list_cmds_by_config().  It
>> sorts the list accumulated so far, uses remove_duplicates() on it
>> without passing free_util=1, so there is also the same kind of leak
>> there, I suspect, until we adjust the call there.
> 
> FWIW, here is what I tentatively queued on top of these four
> patches.  Hopefully we can have a small and final reroll for these
> "Fix small issues in alias" patches and merge them down to 'next'
> soonish?
> 
> Thanks.
> 
> 
> diff --git a/git.c b/git.c
> index c5fad56813..b5eb740e83 100644
> --- a/git.c
> +++ b/git.c
> @@ -119,7 +119,7 @@ static int list_cmds(const char *spec)
>   	}
>   	for (size_t i = 0; i < list.nr; i++)
>   		puts(list.items[i].string);
> -	string_list_clear(&list, 0);
> +	string_list_clear(&list, 1);
>   	return 0;
>   }
>   
> diff --git a/help.c b/help.c
> index 82fb2eaa3f..725e92a195 100644
> --- a/help.c
> +++ b/help.c
> @@ -423,7 +423,7 @@ void list_cmds_by_config(struct string_list *list)
>   		return;
>   
>   	string_list_sort(list);
> -	string_list_remove_duplicates(list, 0);
> +	string_list_remove_duplicates(list, 1);
>   
>   	while (*cmd_list) {
>   		struct strbuf sb = STRBUF_INIT;

This looks correct to me, and I didn't see anything missing. Thanks!