Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()

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

René Scharfe <l.s.r@web.de> writes:

> "These routines have traditionally been used by programs to save the
> name of a working directory for the purpose of returning to it. A much
> faster and less error-prone method of accomplishing this is to open the
> current directory (.) and use the fchdir(2) function to return."
>
> So, how about something like this?

Yeah, I've always wanted to see us use fchdir() for coming back
(another thing I wanted to see is to use openat() and friends).

I do not offhand recall if the run of chdir() in gitdir discovery
code has a similar "now we are done, let's go back to the original"
use of chdir() there, but if we do, we should fix it, too.

Looks sensible from a cursory read.

>
> ---
>  abspath.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/abspath.c b/abspath.c
> index ca33558..7fff13a 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -38,10 +38,10 @@ static const char *real_path_internal(const char *path, int die_on_error)
>  
>  	/*
>  	 * If we have to temporarily chdir(), store the original CWD
> -	 * here so that we can chdir() back to it at the end of the
> +	 * here so that we can fchdir() back to it at the end of the
>  	 * function:
>  	 */
> -	char cwd[1024] = "";
> +	int cwd_fd = -1;
>  
>  	int buf_index = 1;
>  
> @@ -80,7 +80,9 @@ static const char *real_path_internal(const char *path, int die_on_error)
>  		}
>  
>  		if (*buf) {
> -			if (!*cwd && !getcwd(cwd, sizeof(cwd))) {
> +			if (cwd_fd < 0)
> +				cwd_fd = open(".", O_RDONLY);
> +			if (cwd_fd < 0) {
>  				if (die_on_error)
>  					die_errno("Could not get current working directory");
>  				else
> @@ -142,8 +144,11 @@ static const char *real_path_internal(const char *path, int die_on_error)
>  	retval = buf;
>  error_out:
>  	free(last_elem);
> -	if (*cwd && chdir(cwd))
> -		die_errno("Could not change back to '%s'", cwd);
> +	if (cwd_fd >= 0) {
> +		if (fchdir(cwd_fd))
> +			die_errno("Could not change back to the original working directory");
> +		close(cwd_fd);
> +	}
>  
>  	return retval;
>  }