Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 17, 2014, 18:13 UTC
- Message-ID
- <xmqqk37bq2f3.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <53C80265.5030903@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 6 quoted lines
> "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.
Show 47 quoted lines
>
> ---
> 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;
> }