Re: [PATCH 1/9] discover_git_directory(): avoid setting invalid git_dir

From: Brandon Williams <bmwill@google.com>
Date: Jun 7, 2017, 17:45 UTC
Message-ID: <20170607174529.GB110638@google.com>
In-Reply-To: <300d32d72b67f8c12b6cc91c36a5735a77f5ed89.1496851544.git.johannes.schindelin@gmx.de>

On 06/07, Johannes Schindelin wrote:

Show 11 quoted lines

> When discovering a .git/ directory, we take pains to ensure that its
> repository format version matches Git's expectations, and we return NULL
> otherwise.
> 
> However, we still appended the invalid path to the strbuf passed as
> argument.
> 
> Let's just reset the strbuf to the state before we appended the .git/
> directory that was eventually rejected.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Seems sane. This way the strbuf is in the same state it was in before calling this function (upon a failure that is).

Show 14 quoted lines

> ---
>  setup.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/setup.c b/setup.c
> index e3f7699a902..2435186e448 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -982,6 +982,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
>  		warning("ignoring git dir '%s': %s",
>  			gitdir->buf + gitdir_offset, err.buf);
>  		strbuf_release(&err);
> +		strbuf_setlen(gitdir, gitdir_offset);
>  		return NULL;

There is another part of this function that returns NULL (which isn't shown by this diff) after performing 'setup_git_dir_gently_1', do we need to worry about anything that 'setup_git_dir_gently_1' has potentially appended to 'gitdir' upon 'setup_git_dir_gently_1' failing?

Show 6 quoted lines

>  	}
>  
> -- 
> 2.13.0.windows.1.460.g13f583bedb5
> 
>

-- 
Brandon Williams