Re: [PATCH 075/194] fetch, push: do not use submodule as alternate in has_commits check

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 6, 2018, 04:20 UTC
Message-ID: <CAPig+cTjUAYRbaF_CLAmtYf56cL2hhvYc4VLrE8DX-MiryCWXA@mail.gmail.com>
In-Reply-To: <20180205235735.216710-55-sbeller@google.com>

On Mon, Feb 5, 2018 at 6:55 PM, Stefan Beller <sbeller@google.com> wrote:

Show 15 quoted lines

> Both fetch and push still use alternates to access submodules in some
> other code paths, but this is progress towards eliminating the alternates
> hack that conflates access to the_repository and other repositories.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/submodule.c b/submodule.c
> @@ -832,24 +833,43 @@ static int check_has_commit(const struct object_id *oid, void *data)
> +static int open_submodule(struct repository *out, const char *path)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +
> +       if (submodule_to_gitdir(&sb, path))
> +               return -1;

submodule_to_gitdir() makes no promise that it has not made allocations to 'sb' when it returns -1, so this is potentially leaking. Therefore, you should strbuf_release() here.

Show 5 quoted lines

> +
> +       if (repo_init(out, sb.buf, NULL)) {
> +               strbuf_release(&sb);
> +               return -1;
> +       }

Or just combine these two error cases:

    if (submodule_to_gitdir(...) || repo_init(...)) {
        strbuf_release(...);
        return -1;
    }

Show 5 quoted lines

> +       out->submodule_prefix = xstrdup(path);
> +
> +       strbuf_release(&sb);
> +       return 0;
> +}