{"thread":{"id":"37177","subject":"[PATCH v2 0/4] getcwd without PATH_MAX","startedAt":"2014-07-20T16:46:47Z","lastAt":"2014-07-22T10:43:52Z","messageCount":9,"participants":["René Scharfe","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"246419","messageId":"53CBF277.3090101@web.de","threadId":"37177","inReplyTo":null,"subject":"[PATCH v2 0/4] getcwd without PATH_MAX","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-20T16:46:47Z","receivedAt":"2014-07-20T16:46:47Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Paths longer than PATH_MAX can be created and used on at least on some\nfile systems.  Currently we use getcwd() generally with a PATH_MAX-\nsized buffer.  This series adds two functions, strbuf_getcwd() and\nxgetcwd(), then uses them to reduce the number of fixed-sized buffers\nand to allow us to handle longer working directory paths.\n\nNot all getcwd() calls are replaced, however.  I hope that at least\nsome of the remaining ones can be avoided altogether.  If that turns\nout to be too optimistic then we can use the added function to convert\nthe rest.\n\nChanges in v2:\n  * strbuf_getcwd() instead of strbuf_add_cwd(), because the former is\n    simpler and sufficient for now; based on a suggestion by Duy\n  * added patch 2 as an example for strbuf_getcwd() usage, suggested\n    by Duy\n  * made sure strbuf_getcwd() leaves the strbuf intact, no matter\n    what getcwd() does\n  * converted an easy getcwd() call in setup.c\n\nRené Scharfe (4):\n  strbuf: add strbuf_getcwd()\n  use strbuf_getcwd() to get the current working directory without\n    fixed-sized buffers\n  wrapper: add xgetcwd()\n  use xgetcwd() get the current directory or die\n\n Documentation/technical/api-strbuf.txt |  4 ++++\n builtin/init-db.c                      | 25 ++++++++++++-------------\n builtin/rev-parse.c                    |  6 +++---\n dir.c                                  | 12 ++++++++----\n git-compat-util.h                      |  1 +\n git.c                                  |  6 ++++--\n setup.c                                |  6 +++---\n strbuf.c                               | 21 +++++++++++++++++++++\n strbuf.h                               |  1 +\n trace.c                                |  7 ++++---\n wrapper.c                              |  8 ++++++++\n 11 files changed, 69 insertions(+), 28 deletions(-)\n\n-- \n2.0.2\n"},{"id":"246420","messageId":"53CBF311.2000407@web.de","threadId":"37177","inReplyTo":"53CBF277.3090101@web.de","subject":"[PATCH v2 1/4] strbuf: add strbuf_getcwd()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-20T16:49:21Z","receivedAt":"2014-07-20T16:49:21Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add strbuf_getcwd(), which puts the current working directory intto\na strbuf.  Because it doesn't use a fixed-size buffer it supports\narbitrarily long paths, as long as the platform's getcwd() does as\nwell.  At least on Linux and FreeBSD it handles paths longer than\nPATH_MAX just fine.\n\nSuggested-by: Karsten Blees <karsten.blees@gmail.com>\nHelped-by: Helped-by: Duy Nguyen <pclouds@gmail.com>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n Documentation/technical/api-strbuf.txt |  4 ++++\n strbuf.c                               | 21 +++++++++++++++++++++\n strbuf.h                               |  1 +\n 3 files changed, 26 insertions(+)\n\ndiff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\nindex f9c06a7..49e477d 100644\n--- a/Documentation/technical/api-strbuf.txt\n+++ b/Documentation/technical/api-strbuf.txt\n@@ -307,6 +307,10 @@ same behaviour as well.\n \tuse it unless you need the correct position in the file\n \tdescriptor.\n \n+`strbuf_getcwd`::\n+\n+\tSet the buffer to the path of the current working directory.\n+\n `stripspace`::\n \n \tStrip whitespace from a buffer. The second parameter controls if\ndiff --git a/strbuf.c b/strbuf.c\nindex 33018d8..2bf4dfa 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -406,6 +406,27 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)\n \treturn -1;\n }\n \n+int strbuf_getcwd(struct strbuf *sb)\n+{\n+\tsize_t oldalloc = sb->alloc;\n+\tsize_t guessed_len = 128;\n+\n+\tfor (;; guessed_len *= 2) {\n+\t\tstrbuf_grow(sb, guessed_len);\n+\t\tif (getcwd(sb->buf, sb->alloc)) {\n+\t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n+\t\t\treturn 0;\n+\t\t}\n+\t\tif (errno != ERANGE)\n+\t\t\tbreak;\n+\t}\n+\tif (oldalloc == 0)\n+\t\tstrbuf_release(sb);\n+\telse\n+\t\tstrbuf_reset(sb);\n+\treturn -1;\n+}\n+\n int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)\n {\n \tint ch;\ndiff --git a/strbuf.h b/strbuf.h\nindex a7c0192..bc38bb9 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -174,6 +174,7 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);\n extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);\n extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);\n+extern int strbuf_getcwd(struct strbuf *sb);\n \n extern int strbuf_getwholeline(struct strbuf *, FILE *, int);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n-- \n2.0.2\n"},{"id":"246421","messageId":"53CBF332.2050301@web.de","threadId":"37177","inReplyTo":"53CBF277.3090101@web.de","subject":"[PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-20T16:49:54Z","receivedAt":"2014-07-20T16:49:54Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Signed-off-by: Rene Scharfe <l.s.r@web.de>\n---\n builtin/init-db.c | 8 ++++----\n git.c             | 6 ++++--\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 56f85e2..c4958b6 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tusage(init_db_usage[0]);\n \t}\n \tif (is_bare_repository_cfg == 1) {\n-\t\tstatic char git_dir[PATH_MAX+1];\n-\n-\t\tsetenv(GIT_DIR_ENVIRONMENT,\n-\t\t\tgetcwd(git_dir, sizeof(git_dir)), argc > 0);\n+\t\tstruct strbuf cwd = STRBUF_INIT;\n+\t\tstrbuf_getcwd(&cwd);\n+\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, argc > 0);\n+\t\tstrbuf_release(&cwd);\n \t}\n \n \tif (init_shared_repository != -1)\ndiff --git a/git.c b/git.c\nindex 5b6c761..3f52c43 100644\n--- a/git.c\n+++ b/git.c\n@@ -161,9 +161,11 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t} else if (!strcmp(cmd, \"--bare\")) {\n-\t\t\tstatic char git_dir[PATH_MAX+1];\n+\t\t\tstruct strbuf cwd = STRBUF_INIT;\n \t\t\tis_bare_repository_cfg = 1;\n-\t\t\tsetenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);\n+\t\t\tstrbuf_getcwd(&cwd);\n+\t\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, 0);\n+\t\t\tstrbuf_release(&cwd);\n \t\t\tsetenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, \"0\", 1);\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n-- \n2.0.2\n"},{"id":"246422","messageId":"53CBF359.2040400@web.de","threadId":"37177","inReplyTo":"53CBF277.3090101@web.de","subject":"[PATCH v2 3/4] wrapper: add xgetcwd()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-20T16:50:33Z","receivedAt":"2014-07-20T16:50:33Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add the helper function xgetcwd(), which returns the current directory\nor dies.  The returned string has to be free()d after use.\n\nHelped-by: Duy Nguyen <pclouds@gmail.com>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n git-compat-util.h | 1 +\n wrapper.c         | 8 ++++++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 0b53c9a..d64d012 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -605,6 +605,7 @@ extern int xmkstemp(char *template);\n extern int xmkstemp_mode(char *template, int mode);\n extern int odb_mkstemp(char *template, size_t limit, const char *pattern);\n extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);\n+extern char *xgetcwd(void);\n \n static inline size_t xsize_t(off_t len)\n {\ndiff --git a/wrapper.c b/wrapper.c\nindex bc1bfb8..bd24cda 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -493,3 +493,11 @@ struct passwd *xgetpwuid_self(void)\n \t\t    errno ? strerror(errno) : _(\"no such user\"));\n \treturn pw;\n }\n+\n+char *xgetcwd(void)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tif (strbuf_getcwd(&sb))\n+\t\tdie_errno(_(\"unable to get current working directory\"));\n+\treturn strbuf_detach(&sb, NULL);\n+}\n-- \n2.0.2\n"},{"id":"246423","messageId":"53CBF37E.9080205@web.de","threadId":"37177","inReplyTo":"53CBF277.3090101@web.de","subject":"[PATCH v2 4/4] use xgetcwd() get the current directory or die","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-20T16:51:10Z","receivedAt":"2014-07-20T16:51:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Convert several calls of getcwd() and die() to use xgetcwd() instead.\nThis way we get rid of fixed-size buffers (which can be too small\ndepending on the used file system) and gain consistent error messages.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n builtin/init-db.c   | 17 ++++++++---------\n builtin/rev-parse.c |  6 +++---\n dir.c               | 12 ++++++++----\n setup.c             |  6 +++---\n trace.c             |  7 ++++---\n 5 files changed, 26 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex c4958b6..dc226d1 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -426,8 +426,9 @@ int init_db(const char *template_dir, unsigned int flags)\n \n static int guess_repository_type(const char *git_dir)\n {\n-\tchar cwd[PATH_MAX];\n \tconst char *slash;\n+\tchar *cwd;\n+\tint cwd_is_git_dir;\n \n \t/*\n \t * \"GIT_DIR=. git init\" is always bare.\n@@ -435,9 +436,10 @@ static int guess_repository_type(const char *git_dir)\n \t */\n \tif (!strcmp(\".\", git_dir))\n \t\treturn 1;\n-\tif (!getcwd(cwd, sizeof(cwd)))\n-\t\tdie_errno(_(\"cannot tell cwd\"));\n-\tif (!strcmp(git_dir, cwd))\n+\tcwd = xgetcwd();\n+\tcwd_is_git_dir = !strcmp(git_dir, cwd);\n+\tfree(cwd);\n+\tif (cwd_is_git_dir)\n \t\treturn 1;\n \t/*\n \t * \"GIT_DIR=.git or GIT_DIR=something/.git is usually not.\n@@ -572,11 +574,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\tgit_work_tree_cfg = xstrdup(real_path(rel));\n \t\t\tfree(rel);\n \t\t}\n-\t\tif (!git_work_tree_cfg) {\n-\t\t\tgit_work_tree_cfg = xcalloc(PATH_MAX, 1);\n-\t\t\tif (!getcwd(git_work_tree_cfg, PATH_MAX))\n-\t\t\t\tdie_errno (_(\"Cannot access current working directory\"));\n-\t\t}\n+\t\tif (!git_work_tree_cfg)\n+\t\t\tgit_work_tree_cfg = xgetcwd();\n \t\tif (work_tree)\n \t\t\tset_git_work_tree(real_path(work_tree));\n \t\telse\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 8102aaa..f6bbac7 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -735,7 +735,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--git-dir\")) {\n \t\t\t\tconst char *gitdir = getenv(GIT_DIR_ENVIRONMENT);\n-\t\t\t\tstatic char cwd[PATH_MAX];\n+\t\t\t\tchar *cwd;\n \t\t\t\tint len;\n \t\t\t\tif (gitdir) {\n \t\t\t\t\tputs(gitdir);\n@@ -745,10 +745,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tputs(\".git\");\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n-\t\t\t\tif (!getcwd(cwd, PATH_MAX))\n-\t\t\t\t\tdie_errno(\"unable to get current working directory\");\n+\t\t\t\tcwd = xgetcwd();\n \t\t\t\tlen = strlen(cwd);\n \t\t\t\tprintf(\"%s%s.git\\n\", cwd, len && cwd[len-1] != '/' ? \"/\" : \"\");\n+\t\t\t\tfree(cwd);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--resolve-git-dir\")) {\ndiff --git a/dir.c b/dir.c\nindex e65888d..7b994d4 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1499,12 +1499,16 @@ int dir_inside_of(const char *subdir, const char *dir)\n \n int is_inside_dir(const char *dir)\n {\n-\tchar cwd[PATH_MAX];\n+\tchar *cwd;\n+\tint rc;\n+\n \tif (!dir)\n \t\treturn 0;\n-\tif (!getcwd(cwd, sizeof(cwd)))\n-\t\tdie_errno(\"can't find the current directory\");\n-\treturn dir_inside_of(cwd, dir) >= 0;\n+\n+\tcwd = xgetcwd();\n+\trc = (dir_inside_of(cwd, dir) >= 0);\n+\tfree(cwd);\n+\treturn rc;\n }\n \n int is_empty_dir(const char *path)\ndiff --git a/setup.c b/setup.c\nindex 0a22f8b..dc92d63 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -434,16 +434,16 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t\tif (is_absolute_path(git_work_tree_cfg))\n \t\t\tset_git_work_tree(git_work_tree_cfg);\n \t\telse {\n-\t\t\tchar core_worktree[PATH_MAX];\n+\t\t\tchar *core_worktree;\n \t\t\tif (chdir(gitdirenv))\n \t\t\t\tdie_errno(\"Could not chdir to '%s'\", gitdirenv);\n \t\t\tif (chdir(git_work_tree_cfg))\n \t\t\t\tdie_errno(\"Could not chdir to '%s'\", git_work_tree_cfg);\n-\t\t\tif (!getcwd(core_worktree, PATH_MAX))\n-\t\t\t\tdie_errno(\"Could not get directory '%s'\", git_work_tree_cfg);\n+\t\t\tcore_worktree = xgetcwd();\n \t\t\tif (chdir(cwd))\n \t\t\t\tdie_errno(\"Could not come back to cwd\");\n \t\t\tset_git_work_tree(core_worktree);\n+\t\t\tfree(core_worktree);\n \t\t}\n \t}\n \telse if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {\ndiff --git a/trace.c b/trace.c\nindex 08180a9..3523667 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -158,13 +158,12 @@ void trace_repo_setup(const char *prefix)\n {\n \tstatic const char *key = \"GIT_TRACE_SETUP\";\n \tconst char *git_work_tree;\n-\tchar cwd[PATH_MAX];\n+\tchar *cwd;\n \n \tif (!trace_want(key))\n \t\treturn;\n \n-\tif (!getcwd(cwd, PATH_MAX))\n-\t\tdie(\"Unable to get current working directory\");\n+\tcwd = xgetcwd();\n \n \tif (!(git_work_tree = get_git_work_tree()))\n \t\tgit_work_tree = \"(null)\";\n@@ -176,6 +175,8 @@ void trace_repo_setup(const char *prefix)\n \ttrace_printf_key(key, \"setup: worktree: %s\\n\", quote_crnl(git_work_tree));\n \ttrace_printf_key(key, \"setup: cwd: %s\\n\", quote_crnl(cwd));\n \ttrace_printf_key(key, \"setup: prefix: %s\\n\", quote_crnl(prefix));\n+\n+\tfree(cwd);\n }\n \n int trace_want(const char *key)\n-- \n2.0.2\n"},{"id":"246429","messageId":"20140721023312.GC22750@peff.net","threadId":"37177","inReplyTo":"53CBF332.2050301@web.de","subject":"Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-21T02:33:12Z","receivedAt":"2014-07-21T02:33:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 20, 2014 at 06:49:54PM +0200, René Scharfe wrote:\n\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index 56f85e2..c4958b6 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n>  \t\tusage(init_db_usage[0]);\n>  \t}\n>  \tif (is_bare_repository_cfg == 1) {\n> -\t\tstatic char git_dir[PATH_MAX+1];\n> -\n> -\t\tsetenv(GIT_DIR_ENVIRONMENT,\n> -\t\t\tgetcwd(git_dir, sizeof(git_dir)), argc > 0);\n> +\t\tstruct strbuf cwd = STRBUF_INIT;\n> +\t\tstrbuf_getcwd(&cwd);\n> +\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, argc > 0);\n> +\t\tstrbuf_release(&cwd);\n\nHmm. You are not making anything worse here, as we already do not check\nthe return value of getcwd. But what happens if it fails? Looks like we\ncurrently get a segfault, and the new code will silently set the\nvariable to the empty string. Neither is particularly helpful.\n\nShould we be using the xgetcwd helper that you add in the next patch?\n\n> -\t\t\tsetenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);\n> +\t\t\tstrbuf_getcwd(&cwd);\n> +\t\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, 0);\n> +\t\t\tstrbuf_release(&cwd);\n\nDitto here.\n\n-Peff\n"},{"id":"246464","messageId":"53CD4975.8050002@web.de","threadId":"37177","inReplyTo":"20140721023312.GC22750@peff.net","subject":"Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-21T17:10:13Z","receivedAt":"2014-07-21T17:10:13Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 21.07.2014 04:33, schrieb Jeff King:\n> On Sun, Jul 20, 2014 at 06:49:54PM +0200, René Scharfe wrote:\n>\n>> diff --git a/builtin/init-db.c b/builtin/init-db.c\n>> index 56f85e2..c4958b6 100644\n>> --- a/builtin/init-db.c\n>> +++ b/builtin/init-db.c\n>> @@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n>>   \t\tusage(init_db_usage[0]);\n>>   \t}\n>>   \tif (is_bare_repository_cfg == 1) {\n>> -\t\tstatic char git_dir[PATH_MAX+1];\n>> -\n>> -\t\tsetenv(GIT_DIR_ENVIRONMENT,\n>> -\t\t\tgetcwd(git_dir, sizeof(git_dir)), argc > 0);\n>> +\t\tstruct strbuf cwd = STRBUF_INIT;\n>> +\t\tstrbuf_getcwd(&cwd);\n>> +\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, argc > 0);\n>> +\t\tstrbuf_release(&cwd);\n>\n> Hmm. You are not making anything worse here, as we already do not check\n> the return value of getcwd. But what happens if it fails? Looks like we\n> currently get a segfault, and the new code will silently set the\n> variable to the empty string. Neither is particularly helpful.\n>\n> Should we be using the xgetcwd helper that you add in the next patch?\n\nProbably.  And I was so glad to have found an example case for getcwd \nwithout dying and without touching the get-there-and-back cases. :) \nGuess I'll have to look closer at setup.c and perhaps unix-socket.c for \na replacement.\n\nBy the way: Simply setting $GIT_DIR to \".\" probably won't work in the \ntwo cases, I guess?\n\n>\n>> -\t\t\tsetenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);\n>> +\t\t\tstrbuf_getcwd(&cwd);\n>> +\t\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, 0);\n>> +\t\t\tstrbuf_release(&cwd);\n>\n> Ditto here.\n>\n> -Peff\n>\n"},{"id":"246475","messageId":"xmqqsiluilgc.fsf@gitster.dls.corp.google.com","threadId":"37177","inReplyTo":"20140721023312.GC22750@peff.net","subject":"Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-21T19:02:59Z","receivedAt":"2014-07-21T19:02:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Jul 20, 2014 at 06:49:54PM +0200, René Scharfe wrote:\n>\n>> diff --git a/builtin/init-db.c b/builtin/init-db.c\n>> index 56f85e2..c4958b6 100644\n>> --- a/builtin/init-db.c\n>> +++ b/builtin/init-db.c\n>> @@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n>>  \t\tusage(init_db_usage[0]);\n>>  \t}\n>>  \tif (is_bare_repository_cfg == 1) {\n>> -\t\tstatic char git_dir[PATH_MAX+1];\n>> -\n>> -\t\tsetenv(GIT_DIR_ENVIRONMENT,\n>> -\t\t\tgetcwd(git_dir, sizeof(git_dir)), argc > 0);\n>> +\t\tstruct strbuf cwd = STRBUF_INIT;\n>> +\t\tstrbuf_getcwd(&cwd);\n>> +\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, argc > 0);\n>> +\t\tstrbuf_release(&cwd);\n>\n> Hmm. You are not making anything worse here, as we already do not check\n> the return value of getcwd. But what happens if it fails? Looks like we\n> currently get a segfault, and the new code will silently set the\n> variable to the empty string. Neither is particularly helpful.\n>\n> Should we be using the xgetcwd helper that you add in the next patch?\n>\n>> -\t\t\tsetenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);\n>> +\t\t\tstrbuf_getcwd(&cwd);\n>> +\t\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd.buf, 0);\n>> +\t\t\tstrbuf_release(&cwd);\n>\n> Ditto here.\n\nGood eyes.\n"},{"id":"246494","messageId":"20140722104352.GB29607@peff.net","threadId":"37177","inReplyTo":"53CD4975.8050002@web.de","subject":"Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-22T10:43:52Z","receivedAt":"2014-07-22T10:43:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 21, 2014 at 07:10:13PM +0200, René Scharfe wrote:\n\n> Probably.  And I was so glad to have found an example case for getcwd\n> without dying and without touching the get-there-and-back cases. :) Guess\n> I'll have to look closer at setup.c and perhaps unix-socket.c for a\n> replacement.\n\nI think just:\n\n  const char *x = xgetcwd();\n  setenv(GIT_DIR_ENVIRONMENT, x, 0);\n  free(x);\n\nwould be enough?\n\n> By the way: Simply setting $GIT_DIR to \".\" probably won't work in the two\n> cases, I guess?\n\nIt might, but I'd be a little wary. For example, for the call in\ninit_db, would we later then chdir to the working tree in order to do a\ncheckout (since init_db is part of a clone)? Even if it works now, it\nseems like a bit of an accident waiting to happen.\n\n-Peff\n"}]}