From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 14 Jun 2022 11:22:16 GMT
Subject: Re: [PATCH 1/4] branch: add branch_checked_out() helper
Message-ID: <cdf0012a-f104-87e2-104f-db1c8080cf0a@gmail.com>
In-Reply-To: <42ada1a3-adc2-2c43-9aa1-7854858865ae@gmail.com>

On 14/06/2022 11:09, Phillip Wood wrote:
> [..]
>> +static int initialized_checked_out_branches;
>> +static struct strmap current_checked_out_branches = STRMAP_INIT;
> 
> I looks like this map is never freed which I think makes sense but makes 
> me wonder about the relevance of patch 5. I think it would probably be 
> worth marking the map with UNLEAK() in prepare_checked_out_branches().

Actually I think UNLEAK() is for marking stack variables, not globals so 
ignore that. I think patch 5 makes sense in that in keeps the leak bounded.

>> +        if (wt->head_ref)
>> +            strmap_put(&current_checked_out_branches,
>> +                   wt->head_ref,
>> +                   xstrdup(wt->path));
> 
> STRMAP_INIT sets .strdup_strings = 1, so the xstrdup() is unnecessary.

Sorry that's nonsense - it is wt->head_ref that is copied by 
strmap_put(), not wt->path, so the xstrdup() call is correct

Thanks for working on this and sorry for the confusion

Phillip