All of lore.kernel.org
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: "Karsten Blees" <karsten.blees@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
Date: Fri, 18 Jul 2014 13:32:59 +0200	[thread overview]
Message-ID: <53C905EB.3010908@web.de> (raw)
In-Reply-To: <53C8562C.4000304@gmail.com>

Am 18.07.2014 01:03, schrieb Karsten Blees:
> Am 17.07.2014 19:05, schrieb René Scharfe:
>> Am 17.07.2014 14:45, schrieb Nguyễn Thái Ngọc Duy:
> [...]
>> "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."
>>
>
> fchdir() is part of the POSIX-XSI extension, as is realpath(). So why not
> use realpath() directly (which would also be thread-safe)?

That's a good question; thanks for stepping back and looking at the 
bigger picture.  If there is widespread OS support for a functionality 
then we should use it and just provide a compatibility implementation 
for those platforms lacking it.  The downside is that compat code gets 
less testing.

Seeing that readlink() is left as a stub in compat/mingw.h that only 
errors out, would the equivalent function on Windows be PathCanonicalize 
(http://msdn.microsoft.com/en-us/library/windows/desktop/bb773569%28v=vs.85%29.aspx)?

> For non-XSI-compliant platforms, we could keep the current implementation.

OK, so realpath() for Linux and the BSDs, mingw_realpath() wrapping 
PathCanonicalize() for Windows and the current code for the rest?

> Or re-implement a thread-safe version, e.g. applying resolve_symlink() from
> lockfile.c to all path components.

Thread safety sounds good.  We'd also need something like 
normalize_path_copy() but without the conversion of backslashes to 
slashes, in order to get rid of "." and ".." path components and 
something like absolute_path() that doesn't die on error, no?

> If I may bother you with the Windows point of view:
>
> There is no fchdir(), and I'm pretty sure open(".") won't work either.

On Windows, there *is* an absolute path length limit of 260 in the 
normal case and a bit more than 32000 for some functions using the \\?\ 
namespace.  So one could get away with using a constant-sized buffer for 
a "remember the place and return later" function here.

Also, _getcwd can be asked to allocate an appropriately-sized buffer for 
use, like GNU's get_current_dir_name, by specifying NULL as its first 
parameter (http://msdn.microsoft.com/en-us/library/sf98bd4y.aspx).

Not having to move around at all as mentioned above is still better, of 
course.

René

  parent reply	other threads:[~2014-07-18 11:33 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
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 [this message]
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=53C905EB.3010908@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karsten.blees@gmail.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.