getcwd without PATH_MAX
v2 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 series adds two functions, strbuf_getcwd() and xgetcwd(), then uses them to reduce the number of fixed-sized buffers and to allow us to handle longer working directory paths.
Not all getcwd() calls are replaced, however. I hope that at least some of the remaining ones can be avoided altogether. If that turns out to be too optimistic then we can use the added function to convert the rest.
Changes in v2:
* strbuf_getcwd() instead of strbuf_add_cwd(), because the former is simpler and sufficient for now; based on a suggestion by Duy
* added patch 2 as an example for strbuf_getcwd() usage, suggested by Duy
* made sure strbuf_getcwd() leaves the strbuf intact, no matter what getcwd() does
* converted an easy getcwd() call in setup.c
René Scharfe (4):
strbuf: add strbuf_getcwd()
use strbuf_getcwd() to get the current working directory without
fixed-sized buffers
wrapper: add xgetcwd()
use xgetcwd() get the current directory or die
Documentation/technical/api-strbuf.txt | 4 ++++ builtin/init-db.c | 25 ++++++++++++------------- builtin/rev-parse.c | 6 +++--- dir.c | 12 ++++++++---- git-compat-util.h | 1 + git.c | 6 ++++-- setup.c | 6 +++--- strbuf.c | 21 +++++++++++++++++++++ strbuf.h | 1 + trace.c | 7 ++++--- wrapper.c | 8 ++++++++ 11 files changed, 69 insertions(+), 28 deletions(-)
Signature · 1 line
2.0.2
PATCH v2 1/4 strbuf: add strbuf_getcwd()
Add strbuf_getcwd(), which puts the current working directory intto 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 | 21 +++++++++++++++++++++ strbuf.h | 1 + 3 files changed, 26 insertions(+)
Documentation/technical/api-strbuf.txt +4 -0
@@ -307,6 +307,10 @@ same behaviour as well. use it unless you need the correct position in the file descriptor. +`strbuf_getcwd`:: + + Set the buffer to the path of the current working directory. + `stripspace`:: Strip whitespace from a buffer. The second parameter controls if
strbuf.c +21 -0
@@ -406,6 +406,27 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)return -1 ; } +int strbuf_getcwd (struct strbuf *sb) +{ +size_t oldalloc = sb->alloc; +size_t guessed_len =128 ; + +for (;; guessed_len *=2 ) { +strbuf_grow (sb, guessed_len); +if (getcwd (sb->buf, sb->alloc)) { +strbuf_setlen (sb,strlen (sb->buf)); +return 0 ; + } +if (errno !=ERANGE ) +break ; + } +if (oldalloc ==0 ) +strbuf_release (sb); +else +strbuf_reset (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_getcwd (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
PATCH v2 2/4 use strbuf_getcwd() to get the current working directory without fixed-sized buffers
In reply to René Scharfe--- builtin/init-db.c | 8 ++++---- git.c | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-)
builtin/init-db.c +4 -4
@@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)usage (init_db_usage[0 ]); }if (is_bare_repository_cfg ==1 ) { -static char git_dir[PATH_MAX +1 ]; - -setenv (GIT_DIR_ENVIRONMENT , -getcwd (git_dir,sizeof (git_dir)), argc >0 ); +struct strbuf cwd =STRBUF_INIT ; +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf, argc >0 ); +strbuf_release (&cwd); }if (init_shared_repository !=-1 )
git.c +4 -2
@@ -161,9 +161,11 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)if (envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--bare" )) { -static char git_dir[PATH_MAX +1 ]; +struct strbuf cwd =STRBUF_INIT ; is_bare_repository_cfg =1 ; -setenv (GIT_DIR_ENVIRONMENT ,getcwd (git_dir,sizeof (git_dir)),0 ); +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf,0 ); +strbuf_release (&cwd);setenv (GIT_IMPLICIT_WORK_TREE_ENVIRONMENT ,"0" ,1 );if (envchanged) *envchanged =1 ;
Signature · 1 line
2.0.2
Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers
Quoted 6 lines · René Scharfe
builtin/init-db.c +0
-0@@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)usage (init_db_usage[0 ]);
builtin/init-db.c int cmd_init_db(int argc, const char **argv, const char *prefix) · line 541
Quoted 10 lines
builtin/init-db.c +4
-4}if (is_bare_repository_cfg ==1 ) { -static char git_dir[PATH_MAX +1 ]; - -setenv (GIT_DIR_ENVIRONMENT , -getcwd (git_dir,sizeof (git_dir)), argc >0 ); +struct strbuf cwd =STRBUF_INIT ; +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf, argc >0 ); +strbuf_release (&cwd);
Hmm. You are not making anything worse here, as we already do not check the return value of getcwd. But what happens if it fails? Looks like we currently get a segfault, and the new code will silently set the variable to the empty string. Neither is particularly helpful.
Should we be using the xgetcwd helper that you add in the next patch?
builtin/init-db.c int cmd_init_db(int argc, const char **argv, const char *prefix) · line 544
Quoted 4 lines
builtin/init-db.c +3
-1-setenv (GIT_DIR_ENVIRONMENT ,getcwd (git_dir,sizeof (git_dir)),0 ); +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf,0 ); +strbuf_release (&cwd);
Ditto here.
-Peff
Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers
Am 21.07.2014 04:33, schrieb Jeff King:
Quoted 24 lines · René Scharfe
Quoted 7 lines
builtin/init-db.c +0
-0@@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)usage (init_db_usage[0 ]);builtin/init-db.c int cmd_init_db(int argc, const char **argv, const char *prefix) · line 541
Quoted 10 lines
builtin/init-db.c +4
-4}if (is_bare_repository_cfg ==1 ) { -static char git_dir[PATH_MAX +1 ]; - -setenv (GIT_DIR_ENVIRONMENT , -getcwd (git_dir,sizeof (git_dir)), argc >0 ); +struct strbuf cwd =STRBUF_INIT ; +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf, argc >0 ); +strbuf_release (&cwd);Hmm. You are not making anything worse here, as we already do not check the return value of getcwd. But what happens if it fails? Looks like we currently get a segfault, and the new code will silently set the variable to the empty string. Neither is particularly helpful.
Quoted 1 line
Should we be using the xgetcwd helper that you add in the next patch?
Probably. And I was so glad to have found an example case for getcwd without dying and without touching the get-there-and-back cases. :) Guess I'll have to look closer at setup.c and perhaps unix-socket.c for a replacement.
By the way: Simply setting $GIT_DIR to "." probably won't work in the two cases, I guess?
Quoted 10 lines
builtin/init-db.c int cmd_init_db(int argc, const char **argv, const char *prefix) · line 544
Quoted 5 lines
builtin/init-db.c +3
-1-setenv (GIT_DIR_ENVIRONMENT ,getcwd (git_dir,sizeof (git_dir)),0 ); +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf,0 ); +strbuf_release (&cwd);Ditto here.
-Peff
Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers
Quoted 4 lines · René Scharfe
Probably. And I was so glad to have found an example case for getcwd without dying and without touching the get-there-and-back cases. :) Guess I'll have to look closer at
setup.cand perhapsunix-socket.cfor a replacement.
I think just:
const char *x = xgetcwd(); setenv(GIT_DIR_ENVIRONMENT, x, 0); free(x);
would be enough?
Quoted 2 lines
By the way: Simply setting $GIT_DIR to "." probably won't work in the two cases, I guess?
It might, but I'd be a little wary. For example, for the call in init_db, would we later then chdir to the working tree in order to do a checkout (since init_db is part of a clone)? Even if it works now, it seems like a bit of an accident waiting to happen.
-Peff
Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers
In reply to Jeff KingQuoted 24 lines · Jeff King
Quoted 6 lines · René Scharfe
builtin/init-db.c +0
-0@@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)usage (init_db_usage[0 ]);builtin/init-db.c int cmd_init_db(int argc, const char **argv, const char *prefix) · line 541
Quoted 10 lines
builtin/init-db.c +4
-4}if (is_bare_repository_cfg ==1 ) { -static char git_dir[PATH_MAX +1 ]; - -setenv (GIT_DIR_ENVIRONMENT , -getcwd (git_dir,sizeof (git_dir)), argc >0 ); +struct strbuf cwd =STRBUF_INIT ; +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf, argc >0 ); +strbuf_release (&cwd);Hmm. You are not making anything worse here, as we already do not check the return value of getcwd. But what happens if it fails? Looks like we currently get a segfault, and the new code will silently set the variable to the empty string. Neither is particularly helpful.
Quoted 8 lines
Should we be using the xgetcwd helper that you add in the next patch?
builtin/init-db.c int cmd_init_db(int argc, const char **argv, const char *prefix) · line 544
Quoted 4 lines
builtin/init-db.c +3
-1-setenv (GIT_DIR_ENVIRONMENT ,getcwd (git_dir,sizeof (git_dir)),0 ); +strbuf_getcwd (&cwd); +setenv (GIT_DIR_ENVIRONMENT , cwd.buf,0 ); +strbuf_release (&cwd);Ditto here.
Good eyes.
PATCH v2 3/4 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_getcwd (&sb)) +die_errno (_ ("unable to get current working directory" )); +return strbuf_detach (&sb,NULL ); +}
Signature · 1 line
2.0.2
PATCH v2 4/4 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 ++++++++---- setup.c | 6 +++--- trace.c | 7 ++++--- 5 files changed, 26 insertions(+), 22 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)
setup.c +3 -3
@@ -434,16 +434,16 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,if (is_absolute_path (git_work_tree_cfg))set_git_work_tree (git_work_tree_cfg);else { -char core_worktree[PATH_MAX ]; +char *core_worktree;if (chdir (gitdirenv))die_errno ("Could not chdir to '%s'" , gitdirenv);if (chdir (git_work_tree_cfg))die_errno ("Could not chdir to '%s'" , git_work_tree_cfg); -if (!getcwd (core_worktree,PATH_MAX )) -die_errno ("Could not get directory '%s'" , git_work_tree_cfg); + core_worktree =xgetcwd ();if (chdir (cwd))die_errno ("Could not come back to cwd" );set_git_work_tree (core_worktree); +free (core_worktree); } }else if (!git_env_bool (GIT_IMPLICIT_WORK_TREE_ENVIRONMENT ,1 )) {
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