Re: [PATCH 18/23] submodule: use submodule repos for object lookup

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 15, 2018, 19:54 UTC
Message-ID: <20181115195416.21890-1-jonathantanmy@google.com>
In-Reply-To: <20181114001306.138053-19-sbeller@google.com>

Show 10 quoted lines

> +/*
> + * Initialize 'out' based on the provided submodule path.
> + *
> + * Unlike repo_submodule_init, this tolerates submodules not present
> + * in .gitmodules. This function exists only to preserve historical behavior,
> + *
> + * Returns 0 on success, -1 when the submodule is not present.
>   */
> -static void show_submodule_header(struct diff_options *o, const char *path,
> +static struct repository *open_submodule(const char *path)

The function documentation needs to be reworded - there's no "out", and the return value is now a possibly NULL pointer to struct repository.

Show 11 quoted lines

> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	struct repository *out = xmalloc(sizeof(*out));
> +
> +	if (submodule_to_gitdir(&sb, path) || repo_init(out, sb.buf, NULL)) {
> +		strbuf_release(&sb);
> +		free(out);
> +		return NULL;
> +	}
> +
> +	out->submodule_prefix = xstrdup(path);

I've discussed this submodule_prefix line before [1] - do we really need this? Tests pass even if I remove this line.

Other than that, this patch looks good.

[1] https://public-inbox.org/git/20181019203750.110741-1-jonathantanmy@google.com/