Re: [PATCH 1/4] branch: add branch_checked_out() helper

[Phillip Wood <phillip.wood123@xxxxxxxxx>]

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