Re: [PATCH v6 03/12] Prepare setup_discovered_git_directory() the root directory

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 13, 2017, 20:34 UTC
Message-ID: <xmqqr320rk3z.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <8d8be2b095afb215f5bf0e0bb7989c9c9ab3f471.1489435755.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Subject: Re: [PATCH v6 03/12] Prepare setup_discovered_git_directory() the root directory

I do not think you've changed this title throughout the rerolls, but I cannot quite parse it. Would something like this

    setup.c: only append '/' when not at root in setup_discovered_git_dir()

give enough information to the readers while making it readable?

Show 8 quoted lines

> Currently, the offset parameter (indicating what part of the cwd
> parameter corresponds to the current directory after discovering the
> .git/ directory) is set to 0 when we are running in the root directory.
>
> However, in the next patches we will avoid changing the current working
> directory while searching for the .git/ directory, meaning that the
> offset corresponding to the root directory will have to be 1 to reflect
> that this directory is characterized by the path "/" (and not "").

OK. C:\ would be 3 not 1 but the same logic to compare with offset_1st_component() covers the case, which is good.

Show 28 quoted lines

> So let's make sure that setup_discovered_git_directory() only tries to
> append the trailing slash to non-root directories.
>
> Note: the setup_bare_git_directory() does not need a corresponding
> change, as it does not want to return a prefix.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  setup.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 2ac891d4b9a..23114cb7aa3 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -721,8 +721,10 @@ static const char *setup_discovered_git_dir(const char *gitdir,
>  	if (offset == cwd->len)
>  		return NULL;
>  
> -	/* Make "offset" point to past the '/', and add a '/' at the end */
> -	offset++;
> +	/* Make "offset" point past the '/' (already the case for root dirs) */
> +	if (offset != offset_1st_component(cwd->buf))
> +		offset++;
> +	/* Add a '/' at the end */
>  	strbuf_addch(cwd, '/');
>  	return cwd->buf + offset;
>  }