From: Junio C Hamano <gitster@pobox•com>
To: "René Scharfe" <l.s.r@web•de>
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>, git@vger•kernel.org
Subject: Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
Date: Thu, 17 Jul 2014 11:13:20 -0700 [thread overview]
Message-ID: <xmqqk37bq2f3.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <53C80265.5030903@web.de> ("René Scharfe"'s message of "Thu, 17 Jul 2014 19:05:41 +0200")
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;
> }
next prev parent reply other threads:[~2014-07-17 18:13 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-17 12:45 [PATCH] abspath.c: use PATH_MAX in real_path_internal() Nguyễn Thái Ngọc Duy
2014-07-17 17:05 ` René Scharfe
2014-07-17 18:13 ` Junio C Hamano [this message]
2014-07-17 23:03 ` Karsten Blees
2014-07-18 10:49 ` Duy Nguyen
2014-07-18 15:08 ` René Scharfe
2014-07-19 12:51 ` Duy Nguyen
2014-07-20 0:29 ` Karsten Blees
2014-07-20 8:00 ` René Scharfe
2014-07-21 2:25 ` Jeff King
2014-07-18 11:32 ` René Scharfe
2014-07-19 23:55 ` Karsten Blees
2014-07-20 11:17 ` René Scharfe
2014-07-17 18:03 ` Junio C Hamano
2014-07-17 23:02 ` Karsten Blees
2014-07-17 23:03 ` Karsten Blees
2014-07-18 16:45 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqk37bq2f3.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=l.s.r@web$(echo .)de \
--cc=pclouds@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox