Re: [PATCHv5 4/5] worktree: get worktrees from submodules

From: Brandon Williams <bmwill@google.com>
Date: Dec 8, 2016, 01:14 UTC
Message-ID: <20161208011440.GM116201@google.com>
In-Reply-To: <CAGZ79kZiS9dx6QUOcFYh8sSNoVsrv2eNLXJd6X54UekzUiC8VQ@mail.gmail.com>

On 12/07, Stefan Beller wrote:

Show 17 quoted lines

> On Wed, Dec 7, 2016 at 2:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Stefan Beller <sbeller@google.com> writes:
> >
> >> +     submodule_common_dir = strbuf_detach(&sb, NULL);
> >> +     ret = get_worktrees_internal(submodule_common_dir, flags);
> >> +
> >> +     free(submodule_gitdir);
> >
> > This sequence felt somewhat unusual.  I would have written this
> > without an extra variable, i.e.
> >
> >         ret = get_worktrees_internal(sb.buf, flags);
> >         strbuf_release(&sb);
> 
> Yours is cleaner; I don't remember what I was thinking.
> 
> Feel free to squash it in; in case a resend is needed I will do that.

Just make sure to leave that free in as it refers to another variable (submodule_gitdir). It actually turns out there is a memory leak in the original code because submodule_common_dir is never freed after being detached from the strbuf.

-- 
Brandon Williams