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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 8, 2017, 10:00 UTC
Message-ID: <alpine.DEB.2.21.1.1706081143010.171564@virtualbox>
In-Reply-To: <20170607174529.GB110638@google.com>

Hi Brandon,

On Wed, 7 Jun 2017, Brandon Williams wrote:

Show 17 quoted lines

> On 06/07, Johannes Schindelin wrote:
>
> > 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?

The setup_git_dir_gently_1() function used to set the git_dir variable directly in case of success, and leave it unchanged in case of error. My work to allow an early config did not change that, with the exception that it is the gitdir parameter that is changed instead of git_dir.

In other words: that `return NULL` is safe, as it returns due to an error reported by the setup_git_directory_gently_1() function (meaning that the gitdir strbuf has not been touched).

I amended the commit message accordingly, and force-pushed the result.
Meaning: v2 will have that change.

Ciao, Dscho