getcwd without PATH_MAX
cover getcwd without PATH_MAX
Paths longer than PATH_MAX can be created and used on at least on some file systems. Currently we use getcwd() generally with a PATH_MAX- sized buffer. This short series adds two functions, strbuf_add_cwd() and xgetcwd(), then uses them to reduce the number of fixed-sized buffers and to allow us to handle longer working directory paths.
René Scharfe (3):
strbuf: add strbuf_add_cwd()
wrapper: add xgetcwd()
use xgetcwd() get the current directory or die
Documentation/technical/api-strbuf.txt | 4 ++++ builtin/init-db.c | 17 ++++++++--------- builtin/rev-parse.c | 6 +++--- dir.c | 12 ++++++++---- git-compat-util.h | 1 + strbuf.c | 22 ++++++++++++++++++++++ strbuf.h | 1 + trace.c | 7 ++++--- wrapper.c | 8 ++++++++ 9 files changed, 59 insertions(+), 19 deletions(-)
Signature · 1 line
2.0.2
PATCH 1/3 strbuf: add strbuf_add_cwd()
Add strbuf_add_cwd(), which adds the current working directory to a strbuf. Because it doesn't use a fixed-size buffer it supports arbitrarily long paths, as long as the platform's getcwd() does as well. At least on Linux and FreeBSD it handles paths longer than PATH_MAX just fine.
--- Documentation/technical/api-strbuf.txt | 4 ++++ strbuf.c | 22 ++++++++++++++++++++++ strbuf.h | 1 + 3 files changed, 27 insertions(+)
Documentation/technical/api-strbuf.txt +4 -0
@@ -257,6 +257,10 @@ which can be used by the programmer of the callback as she sees fit. Add a formatted string to the buffer. +`strbuf_add_cwd`:: + + Add the current working directory to the buffer. + `strbuf_commented_addf`:: Add a formatted string prepended by a comment character and a
strbuf.c +22 -0
@@ -406,6 +406,28 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)return -1 ; } +int strbuf_add_cwd (struct strbuf *sb) +{ +size_t oldalloc = sb->alloc; +size_t guessed_len =32 ; + +for (;; guessed_len *=2 ) { +char *cwd; + +strbuf_grow (sb, guessed_len); + cwd =getcwd (sb->buf + sb->len, sb->alloc - sb->len); +if (cwd) { +strbuf_setlen (sb, sb->len +strlen (cwd)); +return 0 ; + } +if (errno !=ERANGE ) +break ; + } +if (oldalloc ==0 ) +strbuf_release (sb); +return -1 ; +} +int strbuf_getwholeline (struct strbuf *sb,FILE *fp,int term) {int ch;
strbuf.h +1 -0
@@ -174,6 +174,7 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);extern ssize_t strbuf_read (struct strbuf *,int fd,size_t hint);extern int strbuf_read_file (struct strbuf *sb,const char *path,size_t hint);extern int strbuf_readlink (struct strbuf *sb,const char *path,size_t hint); +extern int strbuf_add_cwd (struct strbuf *sb);extern int strbuf_getwholeline (struct strbuf *,FILE *,int );extern int strbuf_getline (struct strbuf *,FILE *,int );
Signature · 1 line
2.0.2
Re: [PATCH 1/3] strbuf: add strbuf_add_cwd()
strbuf.c int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint) · line 412
Quoted 4 lines · René Scharfe
strbuf.c +4
-0+int strbuf_add_cwd (struct strbuf *sb) +{ +size_t oldalloc = sb->alloc; +size_t guessed_len =32 ;
For Linux, I think this is enough to succesfully get cwd in the first pass. Windows' $HOME is usually deep in C:\Users\Blahblah. Maybe increase this to 128? And we probably want to keep the guessed value, so if the first strbuf_add_cwd needs a few rounds to get cwd, the next strbuf_add_cwd() call does not.
Quoted 7 lines
strbuf.c +7
-0+ +for (;; guessed_len *=2 ) { +char *cwd; + +strbuf_grow (sb, guessed_len); + cwd =getcwd (sb->buf + sb->len, sb->alloc - sb->len); +if (cwd) {
strbuf.c int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint) · line 429
Quoted 10 lines
strbuf.c +10
-0+strbuf_setlen (sb, sb->len +strlen (cwd)); +return 0 ; + } +if (errno !=ERANGE ) +break ; + } +if (oldalloc ==0 ) +strbuf_release (sb); +return -1 ; +}
The loop and the following strbuf_release() are correct. But I wonder if it's easier to read if you save getcwd in a separate/local strbuf variable and only concat it to "sb" when you successfully get cwd..
Signature · 1 line
Duy
Re: [PATCH 1/3] strbuf: add strbuf_add_cwd()
Am 20.07.2014 14:33, schrieb Duy Nguyen:
Quoted 11 lines · René Scharfe
strbuf.c int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint) · line 412
Quoted 4 lines
strbuf.c +4
-0+int strbuf_add_cwd (struct strbuf *sb) +{ +size_t oldalloc = sb->alloc; +size_t guessed_len =32 ;For Linux, I think this is enough to succesfully get cwd in the first pass. Windows' $HOME is usually deep in C:\Users\Blahblah. Maybe increase this to 128? And we probably want to keep the guessed value, so if the first
strbuf_add_cwdneeds a few rounds to get cwd, the nextstrbuf_add_cwd()call does not.
The initial number is debatable, sure. I just copied the 32 from strbuf_readline().
"C:\Users\John Doe\Documents\Projects\foo" (or whatever) isn't thaaat long either, though. FWIW, the longest (non-hidden) path in my home on Windows 8.1 is 139 characters long (as reported by dir -r | %{ $_.FullName.Length } | sort -desc | select -f 1), but there are no git projects in that directory. Not sure if that means 128 would be a better start value, but I don't mind changing it in any case.
Before adding performance optimizations like remembering the last cwd length I'd rather see measurements that demonstrate their use. I doubt we'll see getcwd() show up high on a profile (but didn't measure, except for a run of t/perf).
Quoted 13 lines
Quoted 13 lines
strbuf.c +13
-0+ +for (;; guessed_len *=2 ) { +char *cwd; + +strbuf_grow (sb, guessed_len); + cwd =getcwd (sb->buf + sb->len, sb->alloc - sb->len); +if (cwd) { +strbuf_setlen (sb, sb->len +strlen (cwd)); +return 0 ; + } +if (errno !=ERANGE ) +break ; + }
Quoted 8 lines
strbuf.c int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint) · line 429
Quoted 4 lines
strbuf.c +4
-0+if (oldalloc ==0 ) +strbuf_release (sb); +return -1 ; +}The loop and the following
strbuf_release()are correct. But I wonder if it's easier to read if you save getcwd in a separate/local strbuf variable and only concat it to "sb" when you successfully get cwd..
Adding an extra allocation and string copy sound more complicated. But you are right that the function is more complicated than necessary. Reroll coming..
René
PATCH 2/3 wrapper: add xgetcwd()
In reply to René ScharfeAdd the helper function xgetcwd(), which returns the current directory or dies. The returned string has to be free()d after use.
--- git-compat-util.h | 1 + wrapper.c | 8 ++++++++ 2 files changed, 9 insertions(+)
git-compat-util.h +1 -0
@@ -605,6 +605,7 @@ extern int xmkstemp(char *template);extern int xmkstemp_mode (char *template,int mode);extern int odb_mkstemp (char *template,size_t limit,const char *pattern);extern int odb_pack_keep (char *name,size_t namesz,const unsigned char *sha1); +extern char *xgetcwd (void );static inline size_t xsize_t (off_t len) {
wrapper.c +8 -0
@@ -493,3 +493,11 @@ struct passwd *xgetpwuid_self(void) errno ? strerror(errno) : _("no such user"));return pw; } + +char *xgetcwd (void ) +{ +struct strbuf sb =STRBUF_INIT ; +if (strbuf_add_cwd (&sb)) +die_errno ("unable to get current working directory" ); +return strbuf_detach (&sb,NULL ); +}
Signature · 1 line
2.0.2
Re: [PATCH 2/3] wrapper: add xgetcwd()
wrapper.c struct passwd *xgetpwuid_self(void) · line 501
Quoted 5 lines · René Scharfe
wrapper.c +5
-0+char *xgetcwd (void ) +{ +struct strbuf sb =STRBUF_INIT ; +if (strbuf_add_cwd (&sb)) +die_errno ("unable to get current working directory" );
Wrap the string with _() to make it translatable? I can't see why any script would want to grep this string..
Quoted 2 lines
wrapper.c +2
-0+return strbuf_detach (&sb,NULL ); +}
Signature · 1 line
Duy
Re: [PATCH 2/3] wrapper: add xgetcwd()
Am 20.07.2014 14:35, schrieb Duy Nguyen:
Quoted 9 lines · René Scharfe
wrapper.c struct passwd *xgetpwuid_self(void) · line 501
Quoted 5 lines
wrapper.c +5
-0+char *xgetcwd (void ) +{ +struct strbuf sb =STRBUF_INIT ; +if (strbuf_add_cwd (&sb)) +die_errno ("unable to get current working directory" );Wrap the string with
_()to make it translatable? I can't see why any script would want to grep this string..
Sure, good idea.
Quoted 3 lines
wrapper.c struct passwd *xgetpwuid_self(void) · line 503
Quoted 3 lines
wrapper.c +2
-0+return strbuf_detach (&sb,NULL ); +}
Thank you for the review,
René
PATCH 3/3 use xgetcwd() get the current directory or die
In reply to René ScharfeConvert several calls of getcwd() and die() to use xgetcwd() instead. This way we get rid of fixed-size buffers (which can be too small depending on the used file system) and gain consistent error messages.
--- builtin/init-db.c | 17 ++++++++--------- builtin/rev-parse.c | 6 +++--- dir.c | 12 ++++++++---- trace.c | 7 ++++--- 4 files changed, 23 insertions(+), 19 deletions(-)
builtin/init-db.c +8 -9
@@ -426,8 +426,9 @@ int init_db(const char *template_dir, unsigned int flags)static int guess_repository_type (const char *git_dir) { -char cwd[PATH_MAX ];const char *slash; +char *cwd; +int cwd_is_git_dir;/* * "GIT_DIR=. git init" is always bare.
@@ -435,9 +436,10 @@ static int guess_repository_type(const char *git_dir)*/ if (!strcmp ("." , git_dir))return 1 ; -if (!getcwd (cwd,sizeof (cwd))) -die_errno (_ ("cannot tell cwd" )); -if (!strcmp (git_dir, cwd)) + cwd =xgetcwd (); + cwd_is_git_dir = !strcmp (git_dir, cwd); +free (cwd); +if (cwd_is_git_dir)return 1 ;/* * "GIT_DIR=.git or GIT_DIR=something/.git is usually not.
@@ -572,11 +574,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix) git_work_tree_cfg =xstrdup (real_path (rel));free (rel); } -if (!git_work_tree_cfg) { - git_work_tree_cfg =xcalloc (PATH_MAX ,1 ); -if (!getcwd (git_work_tree_cfg,PATH_MAX )) -die_errno (_ ("Cannot access current working directory" )); - } +if (!git_work_tree_cfg) + git_work_tree_cfg =xgetcwd ();if (work_tree)set_git_work_tree (real_path (work_tree));else
builtin/rev-parse.c +3 -3
@@ -735,7 +735,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) }if (!strcmp (arg,"--git-dir" )) {const char *gitdir =getenv (GIT_DIR_ENVIRONMENT ); -static char cwd[PATH_MAX ]; +char *cwd;int len;if (gitdir) {puts (gitdir);
@@ -745,10 +745,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)puts (".git" );continue ; } -if (!getcwd (cwd,PATH_MAX )) -die_errno ("unable to get current working directory" ); + cwd =xgetcwd (); len =strlen (cwd);printf ("%s%s.git\n" , cwd, len && cwd[len-1 ] !='/' ?"/" :"" ); +free (cwd);continue ; }if (!strcmp (arg,"--resolve-git-dir" )) {
dir.c +8 -4
@@ -1499,12 +1499,16 @@ int dir_inside_of(const char *subdir, const char *dir)int is_inside_dir (const char *dir) { -char cwd[PATH_MAX ]; +char *cwd; +int rc; +if (!dir)return 0 ; -if (!getcwd (cwd,sizeof (cwd))) -die_errno ("can't find the current directory" ); -return dir_inside_of (cwd, dir) >=0 ; + + cwd =xgetcwd (); + rc = (dir_inside_of (cwd, dir) >=0 ); +free (cwd); +return rc; }int is_empty_dir (const char *path)
trace.c +4 -3
@@ -158,13 +158,12 @@ void trace_repo_setup(const char *prefix) {static const char *key ="GIT_TRACE_SETUP" ;const char *git_work_tree; -char cwd[PATH_MAX ]; +char *cwd;if (!trace_want (key))return ; -if (!getcwd (cwd,PATH_MAX )) -die ("Unable to get current working directory" ); + cwd =xgetcwd ();if (!(git_work_tree =get_git_work_tree ())) git_work_tree ="(null)" ;
@@ -176,6 +175,8 @@ void trace_repo_setup(const char *prefix)trace_printf_key (key,"setup: worktree: %s\n" ,quote_crnl (git_work_tree));trace_printf_key (key,"setup: cwd: %s\n" ,quote_crnl (cwd));trace_printf_key (key,"setup: prefix: %s\n" ,quote_crnl (prefix)); + +free (cwd); }int trace_want (const char *key)
Signature · 1 line
2.0.2
Re: [PATCH 3/3] use xgetcwd() get the current directory or die
Quoted 11 lines · René Scharfe
Convert several calls of
getcwd()anddie()to usexgetcwd()instead. This way we get rid of fixed-size buffers (which can be too small depending on the used file system) and gain consistent error messages.Signed-off-by Rene Scharfe <[email protected]>--- builtin/init-db.c | 17 ++++++++--------- builtin/rev-parse.c | 6 +++--- dir.c | 12 ++++++++---- trace.c | 7 ++++--- 4 files changed, 23 insertions(+), 19 deletions(-)
There should be a 4/3 to replace the remaining getcwd with strbuf_getcwd. But I guess we could add that to the low hanging fruit list.
Signature · 1 line
Duy
Re: [PATCH 3/3] use xgetcwd() get the current directory or die
Am 20.07.2014 14:45, schrieb Duy Nguyen:
Quoted 13 lines · René Scharfe
Quoted 11 lines
Convert several calls of
getcwd()anddie()to usexgetcwd()instead. This way we get rid of fixed-size buffers (which can be too small depending on the used file system) and gain consistent error messages.Signed-off-by Rene Scharfe <[email protected]>--- builtin/init-db.c | 17 ++++++++--------- builtin/rev-parse.c | 6 +++--- dir.c | 12 ++++++++---- trace.c | 7 ++++--- 4 files changed, 23 insertions(+), 19 deletions(-)
Quoted 3 lines
There should be a 4/3 to replace the remaining getcwd with
strbuf_getcwd. But I guess we could add that to the low hanging fruit list.
I left out the cases with the go-there-then-come-back pattern on purpose, as they hopefully can be changed to cease using getcwd() in the first place. But I'll include another example (in addition to xgetcwd()) in the reroll.
René