From: Jonathan Nieder <jrnieder@gmail.com>
Date: Thu, 11 Nov 2010 18:17:28 GMT
Subject: Re: [PATCH, maint] setup: make sure git_dir path is in a permanent buffer, getenv(3) case
Message-ID: <20101111181728.GF16972@burratino>
In-Reply-To: <1289498903-18413-1-git-send-email-kirr@mns.spb.ru>

Kirill Smelkov wrote:

> getenv(3) returns not-permanent buffer which may be changed by e.g.
> putenv(3) call (*).

Yikes.  Thanks for the example.

> --- a/environment.c
> +++ b/environment.c
> @@ -88,6 +88,7 @@ const char * const local_repo_env[LOCAL_REPO_ENV_SIZE + 1] = {
>  static void setup_git_env(void)
>  {
>  	git_dir = getenv(GIT_DIR_ENVIRONMENT);
> +	git_dir = git_dir ? xstrdup(git_dir) : NULL;
>  	if (!git_dir) {
>  		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
>  		git_dir = git_dir ? xstrdup(git_dir) : NULL;

Maybe we can avoid (some) repetition like this?

diff --git a/environment.c b/environment.c
index de5581f..942f1e4 100644
--- a/environment.c
+++ b/environment.c
@@ -87,25 +87,31 @@ const char * const local_repo_env[LOCAL_REPO_ENV_SIZE + 1] = {
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
-	if (!git_dir) {
-		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
-		git_dir = git_dir ? xstrdup(git_dir) : NULL;
-	}
 	if (!git_dir)
+		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
+	if (git_dir)
+		git_dir = xstrdup(git_dir);
+	else
 		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+
 	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 (git_object_dir)
+		git_object_dir = xstrdup(git_object_dir);
+	else
+		git_object_dir = git_pathdup("objects");
+
 	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);
-	}
+	if (git_index_file)
+		git_index_file = xstrdup(git_index_file);
+	else
+		git_index_file = git_pathdup("index");
+
 	git_graft_file = getenv(GRAFT_ENVIRONMENT);
-	if (!git_graft_file)
+	if (git_graft_file)
+		git_graft_file = xstrdup(git_graft_file);
+	else
 		git_graft_file = git_pathdup("info/grafts");
+
 	if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
 		read_replace_refs = 0;
 }