Re: [PATCH 10/17] path.c: drop git_path_submodule

[Jeff King <peff@peff.net>]

On Mon, Aug 10, 2015 at 03:57:31PM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jeff King <peff@peff.net> writes:
> >
> >> There are no callers of the slightly-dangerous static-buffer
> >> git_path_submodule left. Let's drop it.
> >
> > There are a few callers added on 'pu', though.
> 
> Actually there is only one.  Here is a proposed evil merge.

Thanks for catching. I (obviously) only checked against 'next' and not
'pu'. Rather than the evil merge, we could also just drop this patch.
And then either leave it be, or fix this one up as a separate topic once
merged. Though it really looks like this call site is potentially
dangerous, with the assignment (I think it is OK, though, because
read_info_alternates does not use git_path itself).

> diff --git a/submodule.c b/submodule.c
> index dfe8b7b..7ab08a1 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -120,34 +120,35 @@ void stage_updated_gitmodules(void)
>  static int add_submodule_odb(const char *path)
> [...]
>  {
>  	struct alternate_object_database *alt_odb;
> -	const char *objects_directory;
> +	struct strbuf objects_directory = STRBUF_INIT;
>  	int ret = 0;
>  
> -	objects_directory = git_path_submodule(path, "objects/");
> -	if (!is_directory(objects_directory)) {
> +	strbuf_git_path_submodule(&objects_directory, "objects/", "%s", path);
> +	if (!is_directory(objects_directory.buf)) {
>  		ret = -1;
>  		goto done;
>  	}

Hmph, the change in 6659e74 would have been a lot nicer if
strbuf_git_path_submodule were already available. Most of what you are
doing here is undoing that commit's strbuf/buf transition. :-/

>  	/* avoid adding it twice */
>  	for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
> -		if (alt_odb->name - alt_odb->base == strlen(objects_directory) &&
> -				!strcmp(alt_odb->base, objects_directory))
> +		if (alt_odb->name - alt_odb->base == objects_directory.len &&
> +				!strcmp(alt_odb->base, objects_directory.buf))
>  			goto done;

Not really relevant to what we're talking about here, but this is the
first time I've lookd at add_submodule_odb. It really looks like the
whole second half of the function could be replaced with a call to
link_alt_odb_entry.

-Peff