{"thread":{"id":"37233","subject":"[PATCH v3 0/10] getcwd without PATH_MAX","startedAt":"2014-07-28T18:21:22Z","lastAt":"2014-08-18T16:50:06Z","messageCount":28,"participants":["René Scharfe","Jeff King","Junio C Hamano","Eric Sunshine","Torsten Bögershausen"],"isPatch":true,"patchVersion":3,"patchTotal":10},"messages":[{"id":"246812","messageId":"53D694A2.8030007@web.de","threadId":"37233","inReplyTo":null,"subject":"[PATCH v3 0/10] getcwd without PATH_MAX","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:21:22Z","receivedAt":"2014-07-28T18:21:22Z","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\nChanges in v3:\n  * all getcwd() calls are converted\n  * the two strbuf_getcwd() examples from last round use xgetcwd()\n    now, as suggested by Jeff\n  * strbuf_add_absolute_path() is introduced\n\nRené Scharfe (10):\n  strbuf: add strbuf_getcwd()\n  unix-sockets: use strbuf_getcwd()\n  setup: convert setup_git_directory_gently_1 et al. to strbuf\n  abspath: use strbuf_getcwd() to remember original working directory\n  abspath: convert real_path_internal() to strbuf\n  wrapper: add xgetcwd()\n  use xgetcwd() to get the current directory or die\n  use xgetcwd() to set $GIT_DIR\n  abspath: convert absolute_path() to strbuf\n  use strbuf_add_absolute_path() to add absolute paths\n\n Documentation/technical/api-strbuf.txt |  10 +++\n abspath.c                              | 124 +++++++++------------------------\n builtin/init-db.c                      |  24 +++----\n builtin/rev-parse.c                    |   6 +-\n dir.c                                  |  12 ++--\n exec_cmd.c                             |   6 +-\n git-compat-util.h                      |   1 +\n git.c                                  |  13 ++--\n setup.c                                |  91 ++++++++++++------------\n sha1_file.c                            |   2 +-\n strbuf.c                               |  46 ++++++++++++\n strbuf.h                               |   3 +\n trace.c                                |   7 +-\n unix-socket.c                          |  14 ++--\n wrapper.c                              |   8 +++\n 15 files changed, 190 insertions(+), 177 deletions(-)\n\n-- \n2.0.2\n"},{"id":"246813","messageId":"53D6955D.4030008@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 01/10] strbuf: add strbuf_getcwd()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:24:29Z","receivedAt":"2014-07-28T18:24:29Z","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 into a\nstrbuf.  Because it doesn't use a fixed-size buffer it supports\narbitrarily long paths, provided the platform's getcwd() does as well.\nAt least on Linux and FreeBSD it handles paths longer than PATH_MAX\njust fine.\n\nSuggested-by: Karsten Blees <karsten.blees@gmail.com>\nHelped-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":"246814","messageId":"53D695A4.2030403@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 02/10] unix-sockets: use strbuf_getcwd()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:25:40Z","receivedAt":"2014-07-28T18:25:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Instead of using a PATH_MAX-sized buffer, which can be too small on some\nfile systems, use strbuf_getcwd(), which handles any path getcwd()\nreturns.  Also preserve the errno set by strbuf_getcwd() instead of\nsetting it to ENAMETOOLONG; that way a more appropriate error message\ncan be shown based on the actual reason for failing.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n unix-socket.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/unix-socket.c b/unix-socket.c\nindex 91bd6b8..19ed48b 100644\n--- a/unix-socket.c\n+++ b/unix-socket.c\n@@ -18,12 +18,12 @@ static int chdir_len(const char *orig, int len)\n }\n \n struct unix_sockaddr_context {\n-\tchar orig_dir[PATH_MAX];\n+\tchar *orig_dir;\n };\n \n static void unix_sockaddr_cleanup(struct unix_sockaddr_context *ctx)\n {\n-\tif (!ctx->orig_dir[0])\n+\tif (!ctx->orig_dir)\n \t\treturn;\n \t/*\n \t * If we fail, we can't just return an error, since we have\n@@ -32,6 +32,7 @@ static void unix_sockaddr_cleanup(struct unix_sockaddr_context *ctx)\n \t */\n \tif (chdir(ctx->orig_dir) < 0)\n \t\tdie(\"unable to restore original working directory\");\n+\tfree(ctx->orig_dir);\n }\n \n static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,\n@@ -39,10 +40,11 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,\n {\n \tint size = strlen(path) + 1;\n \n-\tctx->orig_dir[0] = '\\0';\n+\tctx->orig_dir = NULL;\n \tif (size > sizeof(sa->sun_path)) {\n \t\tconst char *slash = find_last_dir_sep(path);\n \t\tconst char *dir;\n+\t\tstruct strbuf cwd = STRBUF_INIT;\n \n \t\tif (!slash) {\n \t\t\terrno = ENAMETOOLONG;\n@@ -56,11 +58,9 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,\n \t\t\terrno = ENAMETOOLONG;\n \t\t\treturn -1;\n \t\t}\n-\n-\t\tif (!getcwd(ctx->orig_dir, sizeof(ctx->orig_dir))) {\n-\t\t\terrno = ENAMETOOLONG;\n+\t\tif (strbuf_getcwd(&cwd))\n \t\t\treturn -1;\n-\t\t}\n+\t\tctx->orig_dir = strbuf_detach(&cwd, NULL);\n \t\tif (chdir_len(dir, slash - dir) < 0)\n \t\t\treturn -1;\n \t}\n-- \n2.0.2\n"},{"id":"246815","messageId":"53D695E0.2050209@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 03/10] setup: convert setup_git_directory_gently_1 et al. to strbuf","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:26:40Z","receivedAt":"2014-07-28T18:26:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Convert setup_git_directory_gently_1() and its helper functions\nsetup_explicit_git_dir(), setup_discovered_git_dir() and\nsetup_bare_git_dir() to use a struct strbuf to hold the current working\ndirectory.  Replacing the PATH_MAX-sized buffer used before removes a\npath length limition on some file systems.  The functions are converted\nall in one go because they all read and write the variable cwd.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n setup.c | 85 +++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 43 insertions(+), 42 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 0a22f8b..c8b8a97 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -387,7 +387,7 @@ const char *read_gitfile(const char *path)\n }\n \n static const char *setup_explicit_git_dir(const char *gitdirenv,\n-\t\t\t\t\t  char *cwd, int len,\n+\t\t\t\t\t  struct strbuf *cwd,\n \t\t\t\t\t  int *nongit_ok)\n {\n \tconst char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);\n@@ -441,7 +441,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\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\tif (chdir(cwd))\n+\t\t\tif (chdir(cwd->buf))\n \t\t\t\tdie_errno(\"Could not come back to cwd\");\n \t\t\tset_git_work_tree(core_worktree);\n \t\t}\n@@ -459,21 +459,20 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \tworktree = get_git_work_tree();\n \n \t/* both get_git_work_tree() and cwd are already normalized */\n-\tif (!strcmp(cwd, worktree)) { /* cwd == worktree */\n+\tif (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */\n \t\tset_git_dir(gitdirenv);\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n \n-\toffset = dir_inside_of(cwd, worktree);\n+\toffset = dir_inside_of(cwd->buf, worktree);\n \tif (offset >= 0) {\t/* cwd inside worktree? */\n \t\tset_git_dir(real_path(gitdirenv));\n \t\tif (chdir(worktree))\n \t\t\tdie_errno(\"Could not chdir to '%s'\", worktree);\n-\t\tcwd[len++] = '/';\n-\t\tcwd[len] = '\\0';\n+\t\tstrbuf_addch(cwd, '/');\n \t\tfree(gitfile);\n-\t\treturn cwd + offset;\n+\t\treturn cwd->buf + offset;\n \t}\n \n \t/* cwd outside worktree */\n@@ -483,7 +482,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n }\n \n static const char *setup_discovered_git_dir(const char *gitdir,\n-\t\t\t\t\t    char *cwd, int offset, int len,\n+\t\t\t\t\t    struct strbuf *cwd, int offset,\n \t\t\t\t\t    int *nongit_ok)\n {\n \tif (check_repository_format_gently(gitdir, nongit_ok))\n@@ -491,17 +490,17 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \n \t/* --work-tree is set without --git-dir; use discovered one */\n \tif (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {\n-\t\tif (offset != len && !is_absolute_path(gitdir))\n+\t\tif (offset != cwd->len && !is_absolute_path(gitdir))\n \t\t\tgitdir = xstrdup(real_path(gitdir));\n-\t\tif (chdir(cwd))\n+\t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n-\t\treturn setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);\n+\t\treturn setup_explicit_git_dir(gitdir, cwd, nongit_ok);\n \t}\n \n \t/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */\n \tif (is_bare_repository_cfg > 0) {\n-\t\tset_git_dir(offset == len ? gitdir : real_path(gitdir));\n-\t\tif (chdir(cwd))\n+\t\tset_git_dir(offset == cwd->len ? gitdir : real_path(gitdir));\n+\t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n \t\treturn NULL;\n \t}\n@@ -512,18 +511,18 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \t\tset_git_dir(gitdir);\n \tinside_git_dir = 0;\n \tinside_work_tree = 1;\n-\tif (offset == len)\n+\tif (offset == cwd->len)\n \t\treturn NULL;\n \n \t/* Make \"offset\" point to past the '/', and add a '/' at the end */\n \toffset++;\n-\tcwd[len++] = '/';\n-\tcwd[len] = 0;\n-\treturn cwd + offset;\n+\tstrbuf_addch(cwd, '/');\n+\treturn cwd->buf + offset;\n }\n \n /* #16.1, #17.1, #20.1, #21.1, #22.1 (see t1510) */\n-static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongit_ok)\n+static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,\n+\t\t\t\t      int *nongit_ok)\n {\n \tint root_len;\n \n@@ -536,20 +535,20 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi\n \tif (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {\n \t\tconst char *gitdir;\n \n-\t\tgitdir = offset == len ? \".\" : xmemdupz(cwd, offset);\n-\t\tif (chdir(cwd))\n+\t\tgitdir = offset == cwd->len ? \".\" : xmemdupz(cwd->buf, offset);\n+\t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n-\t\treturn setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);\n+\t\treturn setup_explicit_git_dir(gitdir, cwd, nongit_ok);\n \t}\n \n \tinside_git_dir = 1;\n \tinside_work_tree = 0;\n-\tif (offset != len) {\n-\t\tif (chdir(cwd))\n+\tif (offset != cwd->len) {\n+\t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(\"Cannot come back to cwd\");\n-\t\troot_len = offset_1st_component(cwd);\n-\t\tcwd[offset > root_len ? offset : root_len] = '\\0';\n-\t\tset_git_dir(cwd);\n+\t\troot_len = offset_1st_component(cwd->buf);\n+\t\tstrbuf_setlen(cwd, offset > root_len ? offset : root_len);\n+\t\tset_git_dir(cwd->buf);\n \t}\n \telse\n \t\tset_git_dir(\".\");\n@@ -617,10 +616,10 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n {\n \tconst char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);\n \tstruct string_list ceiling_dirs = STRING_LIST_INIT_DUP;\n-\tstatic char cwd[PATH_MAX + 1];\n+\tstatic struct strbuf cwd = STRBUF_INIT;\n \tconst char *gitdirenv, *ret;\n \tchar *gitfile;\n-\tint len, offset, offset_parent, ceil_offset = -1;\n+\tint offset, offset_parent, ceil_offset = -1;\n \tdev_t current_device = 0;\n \tint one_filesystem = 1;\n \n@@ -632,9 +631,9 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \tif (nongit_ok)\n \t\t*nongit_ok = 0;\n \n-\tif (!getcwd(cwd, sizeof(cwd) - 1))\n+\tif (strbuf_getcwd(&cwd))\n \t\tdie_errno(\"Unable to read current working directory\");\n-\toffset = len = strlen(cwd);\n+\toffset = cwd.len;\n \n \t/*\n \t * If GIT_DIR is set explicitly, we're not going\n@@ -643,7 +642,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \t */\n \tgitdirenv = getenv(GIT_DIR_ENVIRONMENT);\n \tif (gitdirenv)\n-\t\treturn setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);\n+\t\treturn setup_explicit_git_dir(gitdirenv, &cwd, nongit_ok);\n \n \tif (env_ceiling_dirs) {\n \t\tint empty_entry_found = 0;\n@@ -651,7 +650,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \t\tstring_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);\n \t\tfilter_string_list(&ceiling_dirs, 0,\n \t\t\t\t   canonicalize_ceiling_entry, &empty_entry_found);\n-\t\tceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);\n+\t\tceil_offset = longest_ancestor_length(cwd.buf, &ceiling_dirs);\n \t\tstring_list_clear(&ceiling_dirs, 0);\n \t}\n \n@@ -683,7 +682,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \n \t\tif (gitdirenv) {\n \t\t\tret = setup_discovered_git_dir(gitdirenv,\n-\t\t\t\t\t\t       cwd, offset, len,\n+\t\t\t\t\t\t       &cwd, offset,\n \t\t\t\t\t\t       nongit_ok);\n \t\t\tfree(gitfile);\n \t\t\treturn ret;\n@@ -691,29 +690,31 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \t\tfree(gitfile);\n \n \t\tif (is_git_directory(\".\"))\n-\t\t\treturn setup_bare_git_dir(cwd, offset, len, nongit_ok);\n+\t\t\treturn setup_bare_git_dir(&cwd, offset, nongit_ok);\n \n \t\toffset_parent = offset;\n-\t\twhile (--offset_parent > ceil_offset && cwd[offset_parent] != '/');\n+\t\twhile (--offset_parent > ceil_offset && cwd.buf[offset_parent] != '/');\n \t\tif (offset_parent <= ceil_offset)\n-\t\t\treturn setup_nongit(cwd, nongit_ok);\n+\t\t\treturn setup_nongit(cwd.buf, nongit_ok);\n \t\tif (one_filesystem) {\n-\t\t\tdev_t parent_device = get_device_or_die(\"..\", cwd, offset);\n+\t\t\tdev_t parent_device = get_device_or_die(\"..\", cwd.buf,\n+\t\t\t\t\t\t\t\toffset);\n \t\t\tif (parent_device != current_device) {\n \t\t\t\tif (nongit_ok) {\n-\t\t\t\t\tif (chdir(cwd))\n+\t\t\t\t\tif (chdir(cwd.buf))\n \t\t\t\t\t\tdie_errno(\"Cannot come back to cwd\");\n \t\t\t\t\t*nongit_ok = 1;\n \t\t\t\t\treturn NULL;\n \t\t\t\t}\n-\t\t\t\tcwd[offset] = '\\0';\n+\t\t\t\tstrbuf_setlen(&cwd, offset);\n \t\t\t\tdie(\"Not a git repository (or any parent up to mount point %s)\\n\"\n-\t\t\t\t\"Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).\", cwd);\n+\t\t\t\t\"Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).\",\n+\t\t\t\t    cwd.buf);\n \t\t\t}\n \t\t}\n \t\tif (chdir(\"..\")) {\n-\t\t\tcwd[offset] = '\\0';\n-\t\t\tdie_errno(\"Cannot change to '%s/..'\", cwd);\n+\t\t\tstrbuf_setlen(&cwd, offset);\n+\t\t\tdie_errno(\"Cannot change to '%s/..'\", cwd.buf);\n \t\t}\n \t\toffset = offset_parent;\n \t}\n-- \n2.0.2\n"},{"id":"246816","messageId":"53D69616.60303@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH 04/10] abspath: use strbuf_getcwd() to remember original working directory","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:27:34Z","receivedAt":"2014-07-28T18:27:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Store the original working directory in a strbuf instead of in a\nfixed-sized buffer, in order to be able to handle longer paths.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n abspath.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex ca33558..911e931 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -41,7 +41,7 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t * here so that we can chdir() back to it at the end of the\n \t * function:\n \t */\n-\tchar cwd[1024] = \"\";\n+\tstruct strbuf cwd = STRBUF_INIT;\n \n \tint buf_index = 1;\n \n@@ -80,7 +80,7 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t\t}\n \n \t\tif (*buf) {\n-\t\t\tif (!*cwd && !getcwd(cwd, sizeof(cwd))) {\n+\t\t\tif (!cwd.len && strbuf_getcwd(&cwd)) {\n \t\t\t\tif (die_on_error)\n \t\t\t\t\tdie_errno(\"Could not get current working directory\");\n \t\t\t\telse\n@@ -142,8 +142,9 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \tretval = buf;\n error_out:\n \tfree(last_elem);\n-\tif (*cwd && chdir(cwd))\n-\t\tdie_errno(\"Could not change back to '%s'\", cwd);\n+\tif (cwd.len && chdir(cwd.buf))\n+\t\tdie_errno(\"Could not change back to '%s'\", cwd.buf);\n+\tstrbuf_release(&cwd);\n \n \treturn retval;\n }\n-- \n2.0.2\n"},{"id":"246818","messageId":"53D6964E.1070100@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 05/10] abspath: convert real_path_internal() to strbuf","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:28:30Z","receivedAt":"2014-07-28T18:28:30Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Use strbuf instead of fixed-sized buffers in real_path() in order to\navoid the size limitations of the latter.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n abspath.c | 69 +++++++++++++++++++++++----------------------------------------\n 1 file changed, 25 insertions(+), 44 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 911e931..16e7fa2 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -33,7 +33,7 @@ int is_directory(const char *path)\n  */\n static const char *real_path_internal(const char *path, int die_on_error)\n {\n-\tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n+\tstatic struct strbuf sb = STRBUF_INIT;\n \tchar *retval = NULL;\n \n \t/*\n@@ -43,14 +43,12 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t */\n \tstruct strbuf cwd = STRBUF_INIT;\n \n-\tint buf_index = 1;\n-\n \tint depth = MAXDEPTH;\n \tchar *last_elem = NULL;\n \tstruct stat st;\n \n \t/* We've already done it */\n-\tif (path == buf || path == next_buf)\n+\tif (path == sb.buf)\n \t\treturn path;\n \n \tif (!*path) {\n@@ -60,26 +58,22 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t\t\tgoto error_out;\n \t}\n \n-\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {\n-\t\tif (die_on_error)\n-\t\t\tdie(\"Too long path: %.*s\", 60, path);\n-\t\telse\n-\t\t\tgoto error_out;\n-\t}\n+\tstrbuf_init(&sb, 0);\n+\tstrbuf_addstr(&sb, path);\n \n \twhile (depth--) {\n-\t\tif (!is_directory(buf)) {\n-\t\t\tchar *last_slash = find_last_dir_sep(buf);\n+\t\tif (!is_directory(sb.buf)) {\n+\t\t\tchar *last_slash = find_last_dir_sep(sb.buf);\n \t\t\tif (last_slash) {\n \t\t\t\tlast_elem = xstrdup(last_slash + 1);\n-\t\t\t\tlast_slash[1] = '\\0';\n+\t\t\t\tstrbuf_setlen(&sb, last_slash - sb.buf + 1);\n \t\t\t} else {\n-\t\t\t\tlast_elem = xstrdup(buf);\n-\t\t\t\t*buf = '\\0';\n+\t\t\t\tlast_elem = xmemdupz(sb.buf, sb.len);\n+\t\t\t\tstrbuf_reset(&sb);\n \t\t\t}\n \t\t}\n \n-\t\tif (*buf) {\n+\t\tif (sb.len) {\n \t\t\tif (!cwd.len && strbuf_getcwd(&cwd)) {\n \t\t\t\tif (die_on_error)\n \t\t\t\t\tdie_errno(\"Could not get current working directory\");\n@@ -87,14 +81,15 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t\t\t\t\tgoto error_out;\n \t\t\t}\n \n-\t\t\tif (chdir(buf)) {\n+\t\t\tif (chdir(sb.buf)) {\n \t\t\t\tif (die_on_error)\n-\t\t\t\t\tdie_errno(\"Could not switch to '%s'\", buf);\n+\t\t\t\t\tdie_errno(\"Could not switch to '%s'\",\n+\t\t\t\t\t\t  sb.buf);\n \t\t\t\telse\n \t\t\t\t\tgoto error_out;\n \t\t\t}\n \t\t}\n-\t\tif (!getcwd(buf, PATH_MAX)) {\n+\t\tif (strbuf_getcwd(&sb)) {\n \t\t\tif (die_on_error)\n \t\t\t\tdie_errno(\"Could not get current working directory\");\n \t\t\telse\n@@ -102,44 +97,30 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t\t}\n \n \t\tif (last_elem) {\n-\t\t\tsize_t len = strlen(buf);\n-\t\t\tif (len + strlen(last_elem) + 2 > PATH_MAX) {\n-\t\t\t\tif (die_on_error)\n-\t\t\t\t\tdie(\"Too long path name: '%s/%s'\",\n-\t\t\t\t\t    buf, last_elem);\n-\t\t\t\telse\n-\t\t\t\t\tgoto error_out;\n-\t\t\t}\n-\t\t\tif (len && !is_dir_sep(buf[len - 1]))\n-\t\t\t\tbuf[len++] = '/';\n-\t\t\tstrcpy(buf + len, last_elem);\n+\t\t\tif (sb.len && !is_dir_sep(sb.buf[sb.len - 1]))\n+\t\t\t\tstrbuf_addch(&sb, '/');\n+\t\t\tstrbuf_addstr(&sb, last_elem);\n \t\t\tfree(last_elem);\n \t\t\tlast_elem = NULL;\n \t\t}\n \n-\t\tif (!lstat(buf, &st) && S_ISLNK(st.st_mode)) {\n-\t\t\tssize_t len = readlink(buf, next_buf, PATH_MAX);\n+\t\tif (!lstat(sb.buf, &st) && S_ISLNK(st.st_mode)) {\n+\t\t\tstruct strbuf next_sb = STRBUF_INIT;\n+\t\t\tssize_t len = strbuf_readlink(&next_sb, sb.buf, 0);\n \t\t\tif (len < 0) {\n \t\t\t\tif (die_on_error)\n-\t\t\t\t\tdie_errno(\"Invalid symlink '%s'\", buf);\n-\t\t\t\telse\n-\t\t\t\t\tgoto error_out;\n-\t\t\t}\n-\t\t\tif (PATH_MAX <= len) {\n-\t\t\t\tif (die_on_error)\n-\t\t\t\t\tdie(\"symbolic link too long: %s\", buf);\n+\t\t\t\t\tdie_errno(\"Invalid symlink '%s'\",\n+\t\t\t\t\t\t  sb.buf);\n \t\t\t\telse\n \t\t\t\t\tgoto error_out;\n \t\t\t}\n-\t\t\tnext_buf[len] = '\\0';\n-\t\t\tbuf = next_buf;\n-\t\t\tbuf_index = 1 - buf_index;\n-\t\t\tnext_buf = bufs[buf_index];\n+\t\t\tstrbuf_swap(&sb, &next_sb);\n+\t\t\tstrbuf_release(&next_sb);\n \t\t} else\n \t\t\tbreak;\n \t}\n \n-\tretval = buf;\n+\tretval = sb.buf;\n error_out:\n \tfree(last_elem);\n \tif (cwd.len && chdir(cwd.buf))\n-- \n2.0.2\n"},{"id":"246817","messageId":"53D6969E.1070202@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 06/10] wrapper: add xgetcwd()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:29:50Z","receivedAt":"2014-07-28T18:29:50Z","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 26e92f1..4d6edea 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -607,6 +607,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":"246819","messageId":"53D696CF.40208@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 07/10] use xgetcwd() to get the current directory or die","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:30:39Z","receivedAt":"2014-07-28T18:30:39Z","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 git.c               |  8 ++++----\n setup.c             |  6 +++---\n trace.c             |  7 ++++---\n 6 files changed, 30 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 56f85e2..f6dd172 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 fcb6872..bd274a7 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1507,12 +1507,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/git.c b/git.c\nindex 9c49519..47137db 100644\n--- a/git.c\n+++ b/git.c\n@@ -20,7 +20,7 @@ const char git_more_info_string[] =\n \n static struct startup_info git_startup_info;\n static int use_pager = -1;\n-static char orig_cwd[PATH_MAX];\n+static char *orig_cwd;\n static const char *env_names[] = {\n \tGIT_DIR_ENVIRONMENT,\n \tGIT_WORK_TREE_ENVIRONMENT,\n@@ -36,8 +36,7 @@ static void save_env(void)\n \tif (saved_environment)\n \t\treturn;\n \tsaved_environment = 1;\n-\tif (!getcwd(orig_cwd, sizeof(orig_cwd)))\n-\t\tdie_errno(\"cannot getcwd\");\n+\torig_cwd = xgetcwd();\n \tfor (i = 0; i < ARRAY_SIZE(env_names); i++) {\n \t\torig_env[i] = getenv(env_names[i]);\n \t\tif (orig_env[i])\n@@ -48,8 +47,9 @@ static void save_env(void)\n static void restore_env(void)\n {\n \tint i;\n-\tif (*orig_cwd && chdir(orig_cwd))\n+\tif (orig_cwd && chdir(orig_cwd))\n \t\tdie_errno(\"could not move to %s\", orig_cwd);\n+\tfree(orig_cwd);\n \tfor (i = 0; i < ARRAY_SIZE(env_names); i++) {\n \t\tif (orig_env[i])\n \t\t\tsetenv(env_names[i], orig_env[i], 1);\ndiff --git a/setup.c b/setup.c\nindex c8b8a97..23b4338 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->buf))\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 e583dc6..54aaee5 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -298,13 +298,12 @@ void trace_repo_setup(const char *prefix)\n {\n \tstatic struct trace_key key = TRACE_KEY_INIT(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@@ -316,6 +315,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(struct trace_key *key)\n-- \n2.0.2\n"},{"id":"246820","messageId":"53D6971D.80107@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 08/10] use xgetcwd() to set $GIT_DIR","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:31:57Z","receivedAt":"2014-07-28T18:31:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Instead of dying of a segmentation fault if getcwd() returns NULL, use\nxgetcwd() to make sure to write a useful error message and then exit\nin an orderly fashion.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n builtin/init-db.c | 7 +++----\n git.c             | 5 +++--\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex f6dd172..ab0ea02 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -537,10 +537,9 @@ 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\tchar *cwd = xgetcwd();\n+\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd, argc > 0);\n+\t\tfree(cwd);\n \t}\n \n \tif (init_shared_repository != -1)\ndiff --git a/git.c b/git.c\nindex 47137db..210f1ae 100644\n--- a/git.c\n+++ b/git.c\n@@ -161,9 +161,10 @@ 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\tchar *cwd = xgetcwd();\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\tsetenv(GIT_DIR_ENVIRONMENT, cwd, 0);\n+\t\t\tfree(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":"246821","messageId":"53D69793.3010307@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 09/10] abspath: convert absolute_path() to strbuf","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:33:55Z","receivedAt":"2014-07-28T18:33:55Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Move most of the code of absolute_path() into the new function\nstrbuf_add_absolute_path() and in the process transform it to use\nstruct strbuf and xgetcwd() instead of a PATH_MAX-sized buffer,\nwhich can be too small on some file systems.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n Documentation/technical/api-strbuf.txt |  6 +++++\n abspath.c                              | 46 +++-------------------------------\n strbuf.c                               | 25 ++++++++++++++++++\n strbuf.h                               |  2 ++\n 4 files changed, 37 insertions(+), 42 deletions(-)\n\ndiff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\nindex 49e477d..430302c 100644\n--- a/Documentation/technical/api-strbuf.txt\n+++ b/Documentation/technical/api-strbuf.txt\n@@ -311,6 +311,12 @@ same behaviour as well.\n \n \tSet the buffer to the path of the current working directory.\n \n+`strbuf_add_absolute_path`\n+\n+\tAdd a path to a buffer, converting a relative path to an\n+\tabsolute one in the process.  Symbolic links are not\n+\tresolved.\n+\n `stripspace`::\n \n \tStrip whitespace from a buffer. The second parameter controls if\ndiff --git a/abspath.c b/abspath.c\nindex 16e7fa2..197af68 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -140,54 +140,16 @@ const char *real_path_if_valid(const char *path)\n \treturn real_path_internal(path, 0);\n }\n \n-static const char *get_pwd_cwd(void)\n-{\n-\tstatic char cwd[PATH_MAX + 1];\n-\tchar *pwd;\n-\tstruct stat cwd_stat, pwd_stat;\n-\tif (getcwd(cwd, PATH_MAX) == NULL)\n-\t\treturn NULL;\n-\tpwd = getenv(\"PWD\");\n-\tif (pwd && strcmp(pwd, cwd)) {\n-\t\tstat(cwd, &cwd_stat);\n-\t\tif ((cwd_stat.st_dev || cwd_stat.st_ino) &&\n-\t\t    !stat(pwd, &pwd_stat) &&\n-\t\t    pwd_stat.st_dev == cwd_stat.st_dev &&\n-\t\t    pwd_stat.st_ino == cwd_stat.st_ino) {\n-\t\t\tstrlcpy(cwd, pwd, PATH_MAX);\n-\t\t}\n-\t}\n-\treturn cwd;\n-}\n-\n /*\n  * Use this to get an absolute path from a relative one. If you want\n  * to resolve links, you should use real_path.\n- *\n- * If the path is already absolute, then return path. As the user is\n- * never meant to free the return value, we're safe.\n  */\n const char *absolute_path(const char *path)\n {\n-\tstatic char buf[PATH_MAX + 1];\n-\n-\tif (!*path) {\n-\t\tdie(\"The empty string is not a valid path\");\n-\t} else if (is_absolute_path(path)) {\n-\t\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n-\t\t\tdie(\"Too long path: %.*s\", 60, path);\n-\t} else {\n-\t\tsize_t len;\n-\t\tconst char *fmt;\n-\t\tconst char *cwd = get_pwd_cwd();\n-\t\tif (!cwd)\n-\t\t\tdie_errno(\"Cannot determine the current working directory\");\n-\t\tlen = strlen(cwd);\n-\t\tfmt = (len > 0 && is_dir_sep(cwd[len - 1])) ? \"%s%s\" : \"%s/%s\";\n-\t\tif (snprintf(buf, PATH_MAX, fmt, cwd, path) >= PATH_MAX)\n-\t\t\tdie(\"Too long path: %.*s\", 60, path);\n-\t}\n-\treturn buf;\n+\tstatic struct strbuf sb;\n+\tstrbuf_init(&sb, 0);\n+\tstrbuf_add_absolute_path(&sb, path);\n+\treturn sb.buf;\n }\n \n /*\ndiff --git a/strbuf.c b/strbuf.c\nindex 2bf4dfa..4d31443 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -576,6 +576,31 @@ void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes)\n \t}\n }\n \n+void strbuf_add_absolute_path(struct strbuf *sb, const char *path)\n+{\n+\tif (!*path)\n+\t\tdie(\"The empty string is not a valid path\");\n+\tif (!is_absolute_path(path)) {\n+\t\tstruct stat cwd_stat, pwd_stat;\n+\t\tsize_t orig_len = sb->len;\n+\t\tchar *cwd = xgetcwd();\n+\t\tchar *pwd = getenv(\"PWD\");\n+\t\tif (pwd && strcmp(pwd, cwd) &&\n+\t\t    !stat(cwd, &cwd_stat) &&\n+\t\t    (cwd_stat.st_dev || cwd_stat.st_ino) &&\n+\t\t    !stat(pwd, &pwd_stat) &&\n+\t\t    pwd_stat.st_dev == cwd_stat.st_dev &&\n+\t\t    pwd_stat.st_ino == cwd_stat.st_ino)\n+\t\t\tstrbuf_addstr(sb, pwd);\n+\t\telse\n+\t\t\tstrbuf_addstr(sb, cwd);\n+\t\tif (sb->len > orig_len && !is_dir_sep(sb->buf[sb->len - 1]))\n+\t\t\tstrbuf_addch(sb, '/');\n+\t\tfree(cwd);\n+\t}\n+\tstrbuf_addstr(sb, path);\n+}\n+\n int printf_ln(const char *fmt, ...)\n {\n \tint ret;\ndiff --git a/strbuf.h b/strbuf.h\nindex bc38bb9..7bdc1da 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -190,6 +190,8 @@ extern void strbuf_addstr_urlencode(struct strbuf *, const char *,\n \t\t\t\t    int reserved);\n extern void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes);\n \n+extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);\n+\n __attribute__((format (printf,1,2)))\n extern int printf_ln(const char *fmt, ...);\n __attribute__((format (printf,2,3)))\n-- \n2.0.2\n"},{"id":"246822","messageId":"53D697C2.8090504@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 10/10] use strbuf_add_absolute_path() to add absolute paths","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:34:42Z","receivedAt":"2014-07-28T18:34:42Z","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 exec_cmd.c  | 6 +-----\n sha1_file.c | 2 +-\n 2 files changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 125fa6f..698e752 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -86,11 +86,7 @@ const char *git_exec_path(void)\n static void add_path(struct strbuf *out, const char *path)\n {\n \tif (path && *path) {\n-\t\tif (is_absolute_path(path))\n-\t\t\tstrbuf_addstr(out, path);\n-\t\telse\n-\t\t\tstrbuf_addstr(out, absolute_path(path));\n-\n+\t\tstrbuf_add_absolute_path(out, path);\n \t\tstrbuf_addch(out, PATH_SEP);\n \t}\n }\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 3f70b1d..95afd20 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -350,7 +350,7 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,\n \t\treturn;\n \t}\n \n-\tstrbuf_addstr(&objdirbuf, absolute_path(get_object_directory()));\n+\tstrbuf_add_absolute_path(&objdirbuf, get_object_directory());\n \tnormalize_path_copy(objdirbuf.buf, objdirbuf.buf);\n \n \talt_copy = xmemdupz(alt, len);\n-- \n2.0.2\n"},{"id":"246823","messageId":"53D69884.7000307@web.de","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"[PATCH v3 04/10] abspath: use strbuf_getcwd() to remember original working directory","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T18:37:56Z","receivedAt":"2014-07-28T18:37:56Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Store the original working directory in a strbuf instead of in a\nfixed-sized buffer, in order to be able to handle longer paths.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\nResent with corrected subject.\n\n abspath.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex ca33558..911e931 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -41,7 +41,7 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t * here so that we can chdir() back to it at the end of the\n \t * function:\n \t */\n-\tchar cwd[1024] = \"\";\n+\tstruct strbuf cwd = STRBUF_INIT;\n \n \tint buf_index = 1;\n \n@@ -80,7 +80,7 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t\t}\n \n \t\tif (*buf) {\n-\t\t\tif (!*cwd && !getcwd(cwd, sizeof(cwd))) {\n+\t\t\tif (!cwd.len && strbuf_getcwd(&cwd)) {\n \t\t\t\tif (die_on_error)\n \t\t\t\t\tdie_errno(\"Could not get current working directory\");\n \t\t\t\telse\n@@ -142,8 +142,9 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \tretval = buf;\n error_out:\n \tfree(last_elem);\n-\tif (*cwd && chdir(cwd))\n-\t\tdie_errno(\"Could not change back to '%s'\", cwd);\n+\tif (cwd.len && chdir(cwd.buf))\n+\t\tdie_errno(\"Could not change back to '%s'\", cwd.buf);\n+\tstrbuf_release(&cwd);\n \n \treturn retval;\n }\n-- \n2.0.2\n"},{"id":"246825","messageId":"20140728185109.GB11265@peff.net","threadId":"37233","inReplyTo":"53D695A4.2030403@web.de","subject":"Re: [PATCH v3 02/10] unix-sockets: use strbuf_getcwd()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-28T18:51:09Z","receivedAt":"2014-07-28T18:51:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 28, 2014 at 08:25:40PM +0200, René Scharfe wrote:\n\n> Instead of using a PATH_MAX-sized buffer, which can be too small on some\n> file systems, use strbuf_getcwd(), which handles any path getcwd()\n> returns.  Also preserve the errno set by strbuf_getcwd() instead of\n> setting it to ENAMETOOLONG; that way a more appropriate error message\n> can be shown based on the actual reason for failing.\n> \n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  unix-socket.c | 14 +++++++-------\n>  1 file changed, 7 insertions(+), 7 deletions(-)\n> \n> diff --git a/unix-socket.c b/unix-socket.c\n> index 91bd6b8..19ed48b 100644\n> --- a/unix-socket.c\n> +++ b/unix-socket.c\n> @@ -18,12 +18,12 @@ static int chdir_len(const char *orig, int len)\n>  }\n>  \n>  struct unix_sockaddr_context {\n> -\tchar orig_dir[PATH_MAX];\n> +\tchar *orig_dir;\n>  };\n\nI would have expected this to just convert to a strbuf. I guess, though\nthat you were making this...\n\n>  \n>  static void unix_sockaddr_cleanup(struct unix_sockaddr_context *ctx)\n>  {\n> -\tif (!ctx->orig_dir[0])\n> +\tif (!ctx->orig_dir)\n>  \t\treturn;\n\n...a little nicer by using the pointer to check for validity. Looks good\nto me.\n\n-Peff\n"},{"id":"246827","messageId":"20140728190946.GC11265@peff.net","threadId":"37233","inReplyTo":"53D6964E.1070100@web.de","subject":"Re: [PATCH v3 05/10] abspath: convert real_path_internal() to strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-28T19:09:46Z","receivedAt":"2014-07-28T19:09:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 28, 2014 at 08:28:30PM +0200, René Scharfe wrote:\n\n>  static const char *real_path_internal(const char *path, int die_on_error)\n>  {\n> -\tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n> +\tstatic struct strbuf sb = STRBUF_INIT;\n\nHrm. I thought at first that this was our usual trick of keeping two\n\"simultaneous\" static buffers, so that we can do:\n\n  printf(\"paths '%s' and '%s'\\n\", real_path(foo), real_path(bar));\n\nBut it looks like that is not the case, and we only have two for\nswapping back and forth as we figure out the answer (but they both need\nto be static, because we do not know which one we will return in the\nend). Is that right?\n\n-Peff\n"},{"id":"246828","messageId":"20140728191500.GD11265@peff.net","threadId":"37233","inReplyTo":"53D69793.3010307@web.de","subject":"Re: [PATCH v3 09/10] abspath: convert absolute_path() to strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-28T19:15:00Z","receivedAt":"2014-07-28T19:15:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 28, 2014 at 08:33:55PM +0200, René Scharfe wrote:\n\n>  const char *absolute_path(const char *path)\n>  {\n> -\tstatic char buf[PATH_MAX + 1];\n> -\n> -\tif (!*path) {\n> -\t\tdie(\"The empty string is not a valid path\");\n> -\t} else if (is_absolute_path(path)) {\n> -\t\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n> -\t\t\tdie(\"Too long path: %.*s\", 60, path);\n> -\t} else {\n> -\t\tsize_t len;\n> -\t\tconst char *fmt;\n> -\t\tconst char *cwd = get_pwd_cwd();\n> -\t\tif (!cwd)\n> -\t\t\tdie_errno(\"Cannot determine the current working directory\");\n> -\t\tlen = strlen(cwd);\n> -\t\tfmt = (len > 0 && is_dir_sep(cwd[len - 1])) ? \"%s%s\" : \"%s/%s\";\n> -\t\tif (snprintf(buf, PATH_MAX, fmt, cwd, path) >= PATH_MAX)\n> -\t\t\tdie(\"Too long path: %.*s\", 60, path);\n> -\t}\n> -\treturn buf;\n> +\tstatic struct strbuf sb;\n> +\tstrbuf_init(&sb, 0);\n> +\tstrbuf_add_absolute_path(&sb, path);\n> +\treturn sb.buf;\n>  }\n\nIs it right to strbuf_init here? That means that we are throwing away\nthe old buffer for each call. I would think you want instead:\n\n  static struct strbuf sb = STRBUF_INIT;\n  strbuf_reset(&sb);\n  ...\n\n-Peff\n"},{"id":"246829","messageId":"20140728191649.GE11265@peff.net","threadId":"37233","inReplyTo":"53D6964E.1070100@web.de","subject":"Re: [PATCH v3 05/10] abspath: convert real_path_internal() to strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-28T19:16:49Z","receivedAt":"2014-07-28T19:16:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 28, 2014 at 08:28:30PM +0200, René Scharfe wrote:\n\n> @@ -60,26 +58,22 @@ static const char *real_path_internal(const char *path, int die_on_error)\n>  \t\t\tgoto error_out;\n>  \t}\n>  \n> -\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {\n> -\t\tif (die_on_error)\n> -\t\t\tdie(\"Too long path: %.*s\", 60, path);\n> -\t\telse\n> -\t\t\tgoto error_out;\n> -\t}\n> +\tstrbuf_init(&sb, 0);\n> +\tstrbuf_addstr(&sb, path);\n\nAs with the other patch I just mentioned, should this be strbuf_reset,\nnot strbuf_init? We want to reset the static buffer back to zero-size,\nnot throw it away and leak whatever was there.\n\n-Peff\n"},{"id":"246830","messageId":"20140728191913.GF11265@peff.net","threadId":"37233","inReplyTo":"53D694A2.8030007@web.de","subject":"Re: [PATCH v3 0/10] getcwd without PATH_MAX","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-28T19:19:13Z","receivedAt":"2014-07-28T19:19:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 28, 2014 at 08:21:22PM +0200, René Scharfe wrote:\n\n> Paths longer than PATH_MAX can be created and used on at least on some\n> file systems.  Currently we use getcwd() generally with a PATH_MAX-\n> sized buffer.  This series adds two functions, strbuf_getcwd() and\n> xgetcwd(), then uses them to reduce the number of fixed-sized buffers\n> and to allow us to handle longer working directory paths.\n\nWith the exception of the potential strbuf_reset/init I pointed out, all\nof these look sane to me.\n\n-Peff\n"},{"id":"246835","messageId":"xmqqiomh40ui.fsf@gitster.dls.corp.google.com","threadId":"37233","inReplyTo":"20140728191649.GE11265@peff.net","subject":"Re: [PATCH v3 05/10] abspath: convert real_path_internal() to strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-28T21:42:13Z","receivedAt":"2014-07-28T21:42:13Z","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 Mon, Jul 28, 2014 at 08:28:30PM +0200, René Scharfe wrote:\n>\n>> @@ -60,26 +58,22 @@ static const char *real_path_internal(const char *path, int die_on_error)\n>>  \t\t\tgoto error_out;\n>>  \t}\n>>  \n>> -\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {\n>> -\t\tif (die_on_error)\n>> -\t\t\tdie(\"Too long path: %.*s\", 60, path);\n>> -\t\telse\n>> -\t\t\tgoto error_out;\n>> -\t}\n>> +\tstrbuf_init(&sb, 0);\n>> +\tstrbuf_addstr(&sb, path);\n>\n> As with the other patch I just mentioned, should this be strbuf_reset,\n> not strbuf_init? We want to reset the static buffer back to zero-size,\n> not throw it away and leak whatever was there.\n>\n> -Peff\n\nYes, this one seems to be leaking.\n\n\"Next call to the function invalidates the return value the last\ncaller received\" feels like playing with fire.  Most existing\ncallers are safe in that the first thing they do to the returned\nstring is xstrdup() it, but we would need to check all the other\ncallers.\n\nI briefly thought it is not OK for set_git_work_tree(), which gets\nnew_work_tree, calls real_path() to receive the value from the\nfunction, and then calls real_path() again on it.  The \"We've\nalready done it\" optimization is the only thing that makes it safe,\nwhich feels overly fragile.\n"},{"id":"246838","messageId":"53D6CCB1.7000809@web.de","threadId":"37233","inReplyTo":"20140728190946.GC11265@peff.net","subject":"Re: [PATCH v3 05/10] abspath: convert real_path_internal() to strbuf","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T22:20:33Z","receivedAt":"2014-07-28T22:20:33Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 28.07.2014 um 21:09 schrieb Jeff King:\n> On Mon, Jul 28, 2014 at 08:28:30PM +0200, René Scharfe wrote:\n>\n>>   static const char *real_path_internal(const char *path, int die_on_error)\n>>   {\n>> -\tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n>> +\tstatic struct strbuf sb = STRBUF_INIT;\n>\n> Hrm. I thought at first that this was our usual trick of keeping two\n> \"simultaneous\" static buffers, so that we can do:\n>\n>    printf(\"paths '%s' and '%s'\\n\", real_path(foo), real_path(bar));\n>\n> But it looks like that is not the case, and we only have two for\n> swapping back and forth as we figure out the answer (but they both need\n> to be static, because we do not know which one we will return in the\n> end). Is that right?\n\nAFAICS it's only swapped to avoid copying the results of a readlink() \ncall against one buffer into the other.  So, yes, that's how I \nunderstand it as well.\n\nRené\n"},{"id":"246839","messageId":"53D6D001.8050304@web.de","threadId":"37233","inReplyTo":"20140728191500.GD11265@peff.net","subject":"Re: [PATCH v3 09/10] abspath: convert absolute_path() to strbuf","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-28T22:34:41Z","receivedAt":"2014-07-28T22:34:41Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 28.07.2014 um 21:15 schrieb Jeff King:\n> On Mon, Jul 28, 2014 at 08:33:55PM +0200, René Scharfe wrote:\n>\n>>   const char *absolute_path(const char *path)\n>>   {\n>> -\tstatic char buf[PATH_MAX + 1];\n>> -\n>> -\tif (!*path) {\n>> -\t\tdie(\"The empty string is not a valid path\");\n>> -\t} else if (is_absolute_path(path)) {\n>> -\t\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n>> -\t\t\tdie(\"Too long path: %.*s\", 60, path);\n>> -\t} else {\n>> -\t\tsize_t len;\n>> -\t\tconst char *fmt;\n>> -\t\tconst char *cwd = get_pwd_cwd();\n>> -\t\tif (!cwd)\n>> -\t\t\tdie_errno(\"Cannot determine the current working directory\");\n>> -\t\tlen = strlen(cwd);\n>> -\t\tfmt = (len > 0 && is_dir_sep(cwd[len - 1])) ? \"%s%s\" : \"%s/%s\";\n>> -\t\tif (snprintf(buf, PATH_MAX, fmt, cwd, path) >= PATH_MAX)\n>> -\t\t\tdie(\"Too long path: %.*s\", 60, path);\n>> -\t}\n>> -\treturn buf;\n>> +\tstatic struct strbuf sb;\n>> +\tstrbuf_init(&sb, 0);\n>> +\tstrbuf_add_absolute_path(&sb, path);\n>> +\treturn sb.buf;\n>>   }\n>\n> Is it right to strbuf_init here? That means that we are throwing away\n> the old buffer for each call. I would think you want instead:\n>\n>    static struct strbuf sb = STRBUF_INIT;\n>    strbuf_reset(&sb);\n>    ...\n\nI changed it from _reset to _init, but I can't remember why. :(  Perhaps \nit's the summer heat.  Your version makes more sense to me now.\n\nRené\n"},{"id":"246860","messageId":"CAPig+cQM2g2LoiwBQxy7ZwbQT0RZt_sKZQahpjJEH7Fn4MyBkg@mail.gmail.com","threadId":"37233","inReplyTo":"53D695E0.2050209@web.de","subject":"Re: [PATCH v3 03/10] setup: convert setup_git_directory_gently_1 et al. to strbuf","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-07-28T23:23:35Z","receivedAt":"2014-07-28T23:23:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 28, 2014 at 2:26 PM, René Scharfe <l.s.r@web.de> wrote:\n> Convert setup_git_directory_gently_1() and its helper functions\n> setup_explicit_git_dir(), setup_discovered_git_dir() and\n> setup_bare_git_dir() to use a struct strbuf to hold the current working\n> directory.  Replacing the PATH_MAX-sized buffer used before removes a\n> path length limition on some file systems.  The functions are converted\n\ns/limition/limitation/\n\n> all in one go because they all read and write the variable cwd.\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n"},{"id":"246862","messageId":"53D6E52B.1050006@web.de","threadId":"37233","inReplyTo":"xmqqiomh40ui.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 05/10] abspath: convert real_path_internal() to strbuf","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-29T00:04:59Z","receivedAt":"2014-07-29T00:04:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 28.07.2014 um 23:42 schrieb Junio C Hamano:\n> Jeff King <peff@peff.net> writes:\n>\n>> On Mon, Jul 28, 2014 at 08:28:30PM +0200, René Scharfe wrote:\n>>\n>>> @@ -60,26 +58,22 @@ static const char *real_path_internal(const char *path, int die_on_error)\n>>>   \t\t\tgoto error_out;\n>>>   \t}\n>>>\n>>> -\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {\n>>> -\t\tif (die_on_error)\n>>> -\t\t\tdie(\"Too long path: %.*s\", 60, path);\n>>> -\t\telse\n>>> -\t\t\tgoto error_out;\n>>> -\t}\n>>> +\tstrbuf_init(&sb, 0);\n>>> +\tstrbuf_addstr(&sb, path);\n>>\n>> As with the other patch I just mentioned, should this be strbuf_reset,\n>> not strbuf_init? We want to reset the static buffer back to zero-size,\n>> not throw it away and leak whatever was there.\n>>\n>> -Peff\n>\n> Yes, this one seems to be leaking.\n>\n> \"Next call to the function invalidates the return value the last\n> caller received\" feels like playing with fire.  Most existing\n> callers are safe in that the first thing they do to the returned\n> string is xstrdup() it, but we would need to check all the other\n> callers.\n\nThat's the price we pay for using static variables, no?  Callers need to \nconsume them as long as they're fresh and multi-threading is not \nallowed.  Before, callers could use wrong buffer contents, after the \npatch they could still have a pointer to freed memory, which should be \nmore noticeable in tests.\n\nGetting a strbuf_add_real_path() in order to avoid static variables \nwould be nice.  And it would also be nice if it worked without calling \nchdir().  Nice topics for follow-up patches. :)\n\n> I briefly thought it is not OK for set_git_work_tree(), which gets\n> new_work_tree, calls real_path() to receive the value from the\n> function, and then calls real_path() again on it.  The \"We've\n> already done it\" optimization is the only thing that makes it safe,\n> which feels overly fragile.\n\nIt wasn't introduced as an optimization, but to silence valgrind \n(1d679de5: make_absolute_path: return the input path if it points to our \nbuffer).  set_git_work_tree() calls real_path() only once in each of its \ntwo branches.  However, one caller (init) hands it a path returned by \nreal_path(); we can change that (sent a patch).\n\nRené\n"},{"id":"246863","messageId":"53D6E530.5000504@web.de","threadId":"37233","inReplyTo":"53D6964E.1070100@web.de","subject":"fixup for 05/10: plug leak","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-29T00:05:04Z","receivedAt":"2014-07-29T00:05:04Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Noticed-by: Jeff King <peff@peff.net>\n---\n abspath.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 16e7fa2..6aa328f 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -58,7 +58,7 @@ static const char *real_path_internal(const char *path, int die_on_error)\n \t\t\tgoto error_out;\n \t}\n \n-\tstrbuf_init(&sb, 0);\n+\tstrbuf_reset(&sb);\n \tstrbuf_addstr(&sb, path);\n \n \twhile (depth--) {\n-- \n2.0.2\n"},{"id":"246864","messageId":"53D6E537.2020001@web.de","threadId":"37233","inReplyTo":"53D69793.3010307@web.de","subject":"fixup for 09/10: plug leak","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-07-29T00:05:11Z","receivedAt":"2014-07-29T00:05:11Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Noticed-by: Jeff King <peff@peff.net>\n---\n abspath.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex cf9b404..5edb4e7 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -146,8 +146,8 @@ const char *real_path_if_valid(const char *path)\n  */\n const char *absolute_path(const char *path)\n {\n-\tstatic struct strbuf sb;\n-\tstrbuf_init(&sb, 0);\n+\tstatic struct strbuf sb = STRBUF_INIT;\n+\tstrbuf_reset(&sb);\n \tstrbuf_add_absolute_path(&sb, path);\n \treturn sb.buf;\n }\n-- \n2.0.2\n"},{"id":"246922","messageId":"xmqqa97s3yjt.fsf@gitster.dls.corp.google.com","threadId":"37233","inReplyTo":"53D6E52B.1050006@web.de","subject":"Re: [PATCH v3 05/10] abspath: convert real_path_internal() to strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-29T16:44:06Z","receivedAt":"2014-07-29T16:44:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n>> \"Next call to the function invalidates the return value the last\n>> caller received\" feels like playing with fire.  Most existing\n>> callers are safe in that the first thing they do to the returned\n>> string is xstrdup() it, but we would need to check all the other\n>> callers.\n>\n> That's the price we pay for using static variables, no?  Callers need\n> to consume them as long as they're fresh and multi-threading is not\n> allowed.\n\nYes, I didn't mean to say that fixing this leak by a static whose\nlifetime rule is \"alive until next call\" is introducing a new\nbrittleness.  The existing callers have lived with that lifetime\nrule with the callee without the changes in this series, and fixing\nthe leak by replacing _init() with _reset() will make the callee\ngive the same old \"alive until next call\" lifetime rule to its\ncallers.\n\n> Getting a strbuf_add_real_path() in order to avoid static variables\n> would be nice.  And it would also be nice if it worked without calling\n> chdir().  Nice topics for follow-up patches. :)\n\nYup.  Nice, but outside the scope.  Of course it is related and can\nbe done as a \"while we know about the issue\" close follow-up.\n\nThanks.\n"},{"id":"247790","messageId":"53EFBB8D.20600@web.de","threadId":"37233","inReplyTo":"53D695E0.2050209@web.de","subject":"Re: [PATCH v3 03/10] setup: convert setup_git_directory_gently_1 et al. to strbuf","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-08-16T20:14:05Z","receivedAt":"2014-08-16T20:14:05Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-07-28 20.26, René Scharfe wrote:\n> Convert setup_git_directory_gently_1() and its helper functions\n> setup_explicit_git_dir(), setup_discovered_git_dir() and\n> setup_bare_git_dir() to use a struct strbuf to hold the current working\n> directory.  Replacing the PATH_MAX-sized buffer used before removes a\n> path length limition on some file systems.  The functions are converted\n> all in one go because they all read and write the variable cwd.\n> \n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  setup.c | 85 +++++++++++++++++++++++++++++++++--------------------------------\n>  1 file changed, 43 insertions(+), 42 deletions(-)\n> \n> diff --git a/setup.c b/setup.c\n> index 0a22f8b..c8b8a97 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -387,7 +387,7 @@ const char *read_gitfile(const char *path)\n>  }\n>  \n>  static const char *setup_explicit_git_dir(const char *gitdirenv,\n> -\t\t\t\t\t  char *cwd, int len,\n> +\t\t\t\t\t  struct strbuf *cwd,\n>  \t\t\t\t\t  int *nongit_ok)\n>  {\n>  \tconst char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);\n> @@ -441,7 +441,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\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\tif (chdir(cwd))\n> +\t\t\tif (chdir(cwd->buf))\n>  \t\t\t\tdie_errno(\"Could not come back to cwd\");\n>  \t\t\tset_git_work_tree(core_worktree);\n>  \t\t}\n> @@ -459,21 +459,20 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n>  \tworktree = get_git_work_tree();\n>  \n>  \t/* both get_git_work_tree() and cwd are already normalized */\n> -\tif (!strcmp(cwd, worktree)) { /* cwd == worktree */\n> +\tif (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */\n>  \t\tset_git_dir(gitdirenv);\n>  \t\tfree(gitfile);\n>  \t\treturn NULL;\n>  \t}\n>  \n> -\toffset = dir_inside_of(cwd, worktree);\n> +\toffset = dir_inside_of(cwd->buf, worktree);\n>  \tif (offset >= 0) {\t/* cwd inside worktree? */\n>  \t\tset_git_dir(real_path(gitdirenv));\n>  \t\tif (chdir(worktree))\n>  \t\t\tdie_errno(\"Could not chdir to '%s'\", worktree);\n> -\t\tcwd[len++] = '/';\n> -\t\tcwd[len] = '\\0';\n> +\t\tstrbuf_addch(cwd, '/');\n>  \t\tfree(gitfile);\n> -\t\treturn cwd + offset;\n> +\t\treturn cwd->buf + offset;\n>  \t}\n>  \n>  \t/* cwd outside worktree */\n> @@ -483,7 +482,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n>  }\n>  \n>  static const char *setup_discovered_git_dir(const char *gitdir,\n> -\t\t\t\t\t    char *cwd, int offset, int len,\n> +\t\t\t\t\t    struct strbuf *cwd, int offset,\n>  \t\t\t\t\t    int *nongit_ok)\n>  {\n>  \tif (check_repository_format_gently(gitdir, nongit_ok))\n> @@ -491,17 +490,17 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n>  \n>  \t/* --work-tree is set without --git-dir; use discovered one */\n>  \tif (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {\n> -\t\tif (offset != len && !is_absolute_path(gitdir))\n> +\t\tif (offset != cwd->len && !is_absolute_path(gitdir))\n>  \t\t\tgitdir = xstrdup(real_path(gitdir));\n> -\t\tif (chdir(cwd))\n> +\t\tif (chdir(cwd->buf))\n>  \t\t\tdie_errno(\"Could not come back to cwd\");\n> -\t\treturn setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);\n> +\t\treturn setup_explicit_git_dir(gitdir, cwd, nongit_ok);\n>  \t}\n>  \n>  \t/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */\n>  \tif (is_bare_repository_cfg > 0) {\n> -\t\tset_git_dir(offset == len ? gitdir : real_path(gitdir));\n> -\t\tif (chdir(cwd))\n> +\t\tset_git_dir(offset == cwd->len ? gitdir : real_path(gitdir));\n> +\t\tif (chdir(cwd->buf))\n>  \t\t\tdie_errno(\"Could not come back to cwd\");\n>  \t\treturn NULL;\n>  \t}\n> @@ -512,18 +511,18 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n>  \t\tset_git_dir(gitdir);\n>  \tinside_git_dir = 0;\n>  \tinside_work_tree = 1;\n> -\tif (offset == len)\n> +\tif (offset == cwd->len)\n>  \t\treturn NULL;\n>  \n>  \t/* Make \"offset\" point to past the '/', and add a '/' at the end */\n>  \toffset++;\n> -\tcwd[len++] = '/';\n> -\tcwd[len] = 0;\n> -\treturn cwd + offset;\n> +\tstrbuf_addch(cwd, '/');\n> +\treturn cwd->buf + offset;\n>  }\n>  \n>  /* #16.1, #17.1, #20.1, #21.1, #22.1 (see t1510) */\n> -static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongit_ok)\n> +static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,\n> +\t\t\t\t      int *nongit_ok)\n>  {\n>  \tint root_len;\n>  \n> @@ -536,20 +535,20 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi\n>  \tif (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {\n>  \t\tconst char *gitdir;\n>  \n> -\t\tgitdir = offset == len ? \".\" : xmemdupz(cwd, offset);\n> -\t\tif (chdir(cwd))\n> +\t\tgitdir = offset == cwd->len ? \".\" : xmemdupz(cwd->buf, offset);\n> +\t\tif (chdir(cwd->buf))\n>  \t\t\tdie_errno(\"Could not come back to cwd\");\n> -\t\treturn setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);\n> +\t\treturn setup_explicit_git_dir(gitdir, cwd, nongit_ok);\n>  \t}\n>  \n>  \tinside_git_dir = 1;\n>  \tinside_work_tree = 0;\n> -\tif (offset != len) {\n> -\t\tif (chdir(cwd))\n> +\tif (offset != cwd->len) {\n> +\t\tif (chdir(cwd->buf))\n>  \t\t\tdie_errno(\"Cannot come back to cwd\");\n> -\t\troot_len = offset_1st_component(cwd);\n> -\t\tcwd[offset > root_len ? offset : root_len] = '\\0';\n> -\t\tset_git_dir(cwd);\n> +\t\troot_len = offset_1st_component(cwd->buf);\n> +\t\tstrbuf_setlen(cwd, offset > root_len ? offset : root_len);\n> +\t\tset_git_dir(cwd->buf);\n>  \t}\n>  \telse\n>  \t\tset_git_dir(\".\");\n> @@ -617,10 +616,10 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n>  {\n>  \tconst char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);\n>  \tstruct string_list ceiling_dirs = STRING_LIST_INIT_DUP;\n> -\tstatic char cwd[PATH_MAX + 1];\n> +\tstatic struct strbuf cwd = STRBUF_INIT;\n\n\nIs there a chance to squueze this in:\n\n\n$ git diff\ndiff --git a/setup.c b/setup.c\nindex 526cdf6..fb61860 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -734,7 +734,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n                string_list_clear(&ceiling_dirs, 0);\n        }\n\n-       if (ceil_offset < 0 && has_dos_drive_prefix(cwd))\n+       if (ceil_offset < 0 && has_dos_drive_prefix(cwd.buf))\n                ceil_offset = 1;\n"},{"id":"247794","messageId":"53EFD1B1.4000909@web.de","threadId":"37233","inReplyTo":"53EFBB8D.20600@web.de","subject":"Re: [PATCH v3 03/10] setup: convert setup_git_directory_gently_1 et al. to strbuf","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-08-16T21:48:33Z","receivedAt":"2014-08-16T21:48:33Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"> Is there a chance to squueze this in:\n> \n> \n> $ git diff\n> diff --git a/setup.c b/setup.c\n> index 526cdf6..fb61860 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -734,7 +734,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n>                  string_list_clear(&ceiling_dirs, 0);\n>          }\n> \n> -       if (ceil_offset < 0 && has_dos_drive_prefix(cwd))\n> +       if (ceil_offset < 0 && has_dos_drive_prefix(cwd.buf))\n>                  ceil_offset = 1;\n> \n> \n\nOuch, thanks for catching this.\n\nPerhaps the following patch should go in as well.\n\n-- >8 --\nSubject: [PATCH] turn path macros into inline function\n\nUse static inline functions instead of macros for has_dos_drive_prefix,\noffset_1st_component, is_dir_sep and find_last_dir_sep in order to let\nthe compiler do type checking.\n\nThe definitions of offset_1st_component and is_dir_sep are switched\naround because the former uses the latter.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n git-compat-util.h | 28 ++++++++++++++++++++++------\n 1 file changed, 22 insertions(+), 6 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f587749..0b6c13a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -264,19 +264,35 @@ extern char *gitbasename(char *);\n #endif\n \n #ifndef has_dos_drive_prefix\n-#define has_dos_drive_prefix(path) 0\n+static inline int git_has_dos_drive_prefix(const char *path)\n+{\n+\treturn 0;\n+}\n+#define has_dos_drive_prefix git_has_dos_drive_prefix\n #endif\n \n-#ifndef offset_1st_component\n-#define offset_1st_component(path) (is_dir_sep((path)[0]))\n+#ifndef is_dir_sep\n+static inline int git_is_dir_sep(int c)\n+{\n+\treturn c == '/';\n+}\n+#define is_dir_sep git_is_dir_sep\n #endif\n \n-#ifndef is_dir_sep\n-#define is_dir_sep(c) ((c) == '/')\n+#ifndef offset_1st_component\n+static inline int git_offset_1st_component(const char *path)\n+{\n+\treturn is_dir_sep(path[0]);\n+}\n+#define offset_1st_component git_offset_1st_component\n #endif\n \n #ifndef find_last_dir_sep\n-#define find_last_dir_sep(path) strrchr(path, '/')\n+static inline char *git_find_last_dir_sep(const char *path)\n+{\n+\treturn strrchr(path, '/');\n+}\n+#define find_last_dir_sep git_find_last_dir_sep\n #endif\n \n #if defined(__HP_cc) && (__HP_cc >= 61000)\n-- \n2.1.0\n"},{"id":"247847","messageId":"xmqqk365yc75.fsf@gitster.dls.corp.google.com","threadId":"37233","inReplyTo":"53EFD1B1.4000909@web.de","subject":"Re: [PATCH v3 03/10] setup: convert setup_git_directory_gently_1 et al. to strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-18T16:50:06Z","receivedAt":"2014-08-18T16:50:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n>> Is there a chance to squueze this in:\n>> \n>> \n>> $ git diff\n>> diff --git a/setup.c b/setup.c\n>> index 526cdf6..fb61860 100644\n>> --- a/setup.c\n>> +++ b/setup.c\n>> @@ -734,7 +734,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n>>                  string_list_clear(&ceiling_dirs, 0);\n>>          }\n>> \n>> -       if (ceil_offset < 0 && has_dos_drive_prefix(cwd))\n>> +       if (ceil_offset < 0 && has_dos_drive_prefix(cwd.buf))\n>>                  ceil_offset = 1;\n>> \n>> \n>\n> Ouch, thanks for catching this.\n\nLet me squash it in to the original change when rebuilding 'next'\nsometime this week, since we have tagged 2.1 now.\n\n> Perhaps the following patch should go in as well.\n\nNice; it catches the above, and shows there isn't any similar gotcha\nat least for me (meaning: parts inside e.g. \"#ifdef WINDOWS\" that I\ndo not compile are not covered by \"at least for me\" test I did).\n\nThanks.\n\n> -- >8 --\n> Subject: [PATCH] turn path macros into inline function\n>\n> Use static inline functions instead of macros for has_dos_drive_prefix,\n> offset_1st_component, is_dir_sep and find_last_dir_sep in order to let\n> the compiler do type checking.\n>\n> The definitions of offset_1st_component and is_dir_sep are switched\n> around because the former uses the latter.\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  git-compat-util.h | 28 ++++++++++++++++++++++------\n>  1 file changed, 22 insertions(+), 6 deletions(-)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index f587749..0b6c13a 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -264,19 +264,35 @@ extern char *gitbasename(char *);\n>  #endif\n>  \n>  #ifndef has_dos_drive_prefix\n> -#define has_dos_drive_prefix(path) 0\n> +static inline int git_has_dos_drive_prefix(const char *path)\n> +{\n> +\treturn 0;\n> +}\n> +#define has_dos_drive_prefix git_has_dos_drive_prefix\n>  #endif\n>  \n> -#ifndef offset_1st_component\n> -#define offset_1st_component(path) (is_dir_sep((path)[0]))\n> +#ifndef is_dir_sep\n> +static inline int git_is_dir_sep(int c)\n> +{\n> +\treturn c == '/';\n> +}\n> +#define is_dir_sep git_is_dir_sep\n>  #endif\n>  \n> -#ifndef is_dir_sep\n> -#define is_dir_sep(c) ((c) == '/')\n> +#ifndef offset_1st_component\n> +static inline int git_offset_1st_component(const char *path)\n> +{\n> +\treturn is_dir_sep(path[0]);\n> +}\n> +#define offset_1st_component git_offset_1st_component\n>  #endif\n>  \n>  #ifndef find_last_dir_sep\n> -#define find_last_dir_sep(path) strrchr(path, '/')\n> +static inline char *git_find_last_dir_sep(const char *path)\n> +{\n> +\treturn strrchr(path, '/');\n> +}\n> +#define find_last_dir_sep git_find_last_dir_sep\n>  #endif\n>  \n>  #if defined(__HP_cc) && (__HP_cc >= 61000)\n"}]}