Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf
From: Jeff King <hidden>
Date: 2016-06-15 23:01:44
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf · Jeff King <hidden>
- 2016-06-15 · Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf · Junio C Hamano <hidden>
On Tue, Jun 24, 2014 at 08:30:26PM +0700, Duy Nguyen wrote:
On Fri, Jun 20, 2014 at 4:28 AM, Jeff King [off-list ref] wrote:quoted
diff --git a/environment.c b/environment.c index 4dac5e9..4de7b81 100644 --- a/environment.c +++ b/environment.c@@ -135,15 +135,11 @@ static void setup_git_env(void) gitfile = read_gitfile(git_dir); git_dir = xstrdup(gitfile ? gitfile : git_dir); git_object_dir = getenv(DB_ENVIRONMENT); - if (!git_object_dir) { - git_object_dir = xmalloc(strlen(git_dir) + 9); - sprintf(git_object_dir, "%s/objects", git_dir); - }If DB_ENVIRONMENT is set, we should xstrdup(git_object_dir) because getenv's return value is not guaranteed persistent. Since you're touch this area, perhaps do it too (in this, or another patch)?
Here's a replacement patch that handles this (and just drops the ugly mallocs as a side effect). -- >8 -- Subject: [PATCH] setup_git_env: copy getenv return value The return value of getenv is not guaranteed to survive across further invocations of setenv or even getenv. When we are assigning it to globals that last the lifetime of the program, we should make our own copy. Signed-off-by: Jeff King <redacted> --- environment.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/environment.c b/environment.c
index 4dac5e9..efb2fa0 100644
--- a/environment.c
+++ b/environment.c@@ -124,6 +124,12 @@ static char *expand_namespace(const char *raw_namespace) return strbuf_detach(&buf, NULL); } +static char *git_path_from_env(const char *envvar, const char *path) +{ + const char *value = getenv(envvar); + return value ? xstrdup(value) : git_pathdup(path); +} + static void setup_git_env(void) { const char *gitfile;
@@ -134,19 +140,9 @@ static void setup_git_env(void) git_dir = DEFAULT_GIT_DIR_ENVIRONMENT; gitfile = read_gitfile(git_dir); git_dir = xstrdup(gitfile ? gitfile : git_dir); - git_object_dir = getenv(DB_ENVIRONMENT); - if (!git_object_dir) { - git_object_dir = xmalloc(strlen(git_dir) + 9); - sprintf(git_object_dir, "%s/objects", git_dir); - } - git_index_file = getenv(INDEX_ENVIRONMENT); - if (!git_index_file) { - git_index_file = xmalloc(strlen(git_dir) + 7); - sprintf(git_index_file, "%s/index", git_dir); - } - git_graft_file = getenv(GRAFT_ENVIRONMENT); - if (!git_graft_file) - git_graft_file = git_pathdup("info/grafts"); + git_object_dir = git_path_from_env(DB_ENVIRONMENT, "objects"); + git_index_file = git_path_from_env(INDEX_ENVIRONMENT, "index"); + git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, "info/grafts"); if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT)) check_replace_refs = 0; namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
--
2.0.0.566.gfe3e6b2