Re: [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves

From: Stefan Beller <sbeller@google.com>
Date: Jan 25, 2017, 23:04 UTC
Message-ID: <CAGZ79kaSQ8YE2S2MuaaOo9HeyHL7A66Ug9aO_j7F=o2e5RO6iQ@mail.gmail.com>
In-Reply-To: <20170125004610.GC58021@google.com>

On Tue, Jan 24, 2017 at 4:46 PM, Brandon Williams <bmwill@google.com> wrote:

> On 01/24, Stefan Beller wrote:
>> +             connect_work_tree_and_git_dir(path, real_new_git_dir);
>
> Memory leak with 'real_new_git_dir'

yup. :(

> The scope of 'real_sub_git_dir' and 'real_common_git_dir' variable can
> be narrowed.  Move their declaration here and free it prior to exiting
> the else block.

ok.

> 'v' isn't ever used, just use starts_with() and get rid of 'v'

makes sense.

Show 7 quoted lines

>
>> +                     relocate_single_git_dir_into_superproject(prefix, path);
>> +     }
>>
>
> There's a label 'out' at the end of the function which can be removed as
> there is no 'goto' statement using it.

We do use it in

    if (err_code == READ_GITFILE_ERR_STAT_FAILED)
        goto out; /* unpopulated as expected */

I took your proposed SQUASH and removed the goto, see the followup email.