Re: [PATCH 1/4] set_git_dir: fix crash when used with real_path()

[Junio C Hamano <gitster@pobox.com>]

"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
>
> `real_path()` returns result from a shared buffer, inviting subtle
> reentrance bugs. One of these bugs occur when invoked this way:
>     set_git_dir(real_path(git_dir))
>
> In this case, `real_path()` has reentrance:
>     real_path
>     read_gitfile_gently
>     repo_set_gitdir
>     setup_git_env
>     set_git_dir_1
>     set_git_dir
>
> Later, `set_git_dir()` uses its now-dead parameter:
>     !is_absolute_path(path)
>
> Fix this by using a dedicated `strbuf` to hold `strbuf_realpath()`.

With this detailed explanation, I expected to see a test or two that
demonstrates a breakage, but reading a stale value may not
reproducibly give the same wrong result or crash the program,
perhaps?

> -void set_git_dir(const char *path)
> +void set_git_dir(const char *path, int make_realpath)
>  {
> +	struct strbuf realpath = STRBUF_INIT;
> +
> +	if (make_realpath) {
> +		strbuf_realpath(&realpath, path, 1);
> +		path = realpath.buf;
> +	}
> +
>  	set_git_dir_1(path);
>  	if (!is_absolute_path(path))
>  		chdir_notify_register(NULL, update_relative_gitdir, NULL);
> +
> +	strbuf_release(&realpath);
>  }

Makes sense.  I looked at changes to the callers in this patch and
it all made sense.