Re: [PATCH] add_submodule_odb: initialize alt_odb list earlier

> Yeah, I can reproduce it easily with that. Thanks for providing the
> repository. It takes a rather convoluted set of conditions to trigger
> the bug. :)
>
> Here's the fix:
>
> -- >8 --
> Subject: add_submodule_odb: initialize alt_odb list earlier
>
> The add_submodule_odb function tries to add a submodule's
> object store as an "alternate". It needs the existing list
> to be initialized (from the objects/info/alternates file)
> for two reasons:
>
>   1. We look for duplicates with the existing alternate
>      stores, but obviously this doesn't work if we haven't
>      loaded any yet.
>
>   2. We link our new entry into the list by prepending it to
>      alt_odb_list. But we do _not_ modify alt_odb_tail.
>      This variable starts as NULL, and is a signal to the
>      alt_odb code that the list has not yet been
>      initialized.
>
>      We then call read_info_alternates on the submodule (to
>      recursively load its alternates), which will try to
>      append to that tail, assuming it has been initialized.
>      This causes us to segfault if it is NULL.
>
> This rarely comes up in practice, because we will have
> initialized the alt_odb any time we do an object lookup. So
> you can trigger this only when:
>
>   - you try to access a submodule (e.g., a diff with
>     diff.submodule=log)
>
>   - the access happens before any other object has been
>     accessed (e.g., because the diff is between the working
>     tree and the index)
>
>   - the submodule contains an alternates file (so we try to
>     add an entry to the NULL alt_odb_tail)
>
> To fix this, we just need to call prepare_alt_odb at the
> start of the function (and if we have already initialized,
> it is a noop).
>
> Note that we can remove the prepare_alt_odb call from the
> end. It is guaranteed to be a noop, since we will have
> called it earlier.

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  submodule.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/submodule.c b/submodule.c
> index 5879cfb..88af54c 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -130,6 +130,7 @@ static int add_submodule_odb(const char *path)
>  		goto done;
>  	}
>  	/* avoid adding it twice */
> +	prepare_alt_odb();
>  	for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
>  		if (alt_odb->name - alt_odb->base == objects_directory.len &&
>  				!strncmp(alt_odb->base, objects_directory.buf,
> @@ -148,7 +149,6 @@ static int add_submodule_odb(const char *path)
>  
>  	/* add possible alternates from the submodule */
>  	read_info_alternates(objects_directory.buf, 0);
> -	prepare_alt_odb();
>  done:
>  	strbuf_release(&objects_directory);
>  	return ret;