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

[Brandon Williams]

On 12/07, Stefan Beller wrote:
> On Wed, Dec 7, 2016 at 2:45 PM, Junio C Hamano  wrote:
> > Stefan Beller  writes:
> >
> >> + submodule_common_dir = strbuf_detach(, 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();
> 
> 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

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

[Stefan Beller]

On Wed, Dec 7, 2016 at 2:45 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> + submodule_common_dir = strbuf_detach(, 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();

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.

Thanks,
Stefan

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

[Junio C Hamano]

Stefan Beller  writes:

> + submodule_common_dir = strbuf_detach(, 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();