Fix git's usage of getenv() on systems with exotic getenv implementations

According to the POSIX specification, "The return value from getenv() may
point to static data which may be overwritten by subsequent calls to
getenv(), setenv(), [putenv()] or unsetenv()." and "The getenv() function
need not be reentrant." [1]

Many getenv() usages in git currently require that the returned values
remain valid at least across subsequent getenv() calls. In some cases
getenv() is used in background threads. This only works with typical
getenv() implementations that return pointers into the char **environ
array.

As demonstrated by Jonathan Nieder's GETENV_POISON patch [2], more than
half of git's test-suite fails with a most hazardous, yet POSIX compliant
getenv() implementation that overwrites its own return values.

This patch makes git's getenv() usage fully POSIX compliant, without having
to fix the many getenv() references throughout git. This is achieved by
overriding getenv() with a version that is thread-safe and ensures that all
returned values remain valid until the process ends. Identical return
values are unified in a hash table to save memory.

The functionality can be enabled on platforms that require it by defining
UNSAFE_GETENV in the Makefile.

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/getenv.html
[2] http://article.gmane.org/gmane.comp.version-control.git/161356

Signed-off-by: Karsten Blees <[email protected]>

Author: kblees

Makefile

@@ -242,6 +242,9 @@ all::
 # dependency rules.
 #
 # Define NATIVE_CRLF if your platform uses CRLF for line endings.
+#
+# Define UNSAFE_GETENV if getenv isn't thread-safe or overwrites it's own
+# return values.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1409,6 +1412,10 @@ ifdef NO_SETENV
 	COMPAT_CFLAGS += -DNO_SETENV
 	COMPAT_OBJS += compat/setenv.o
 endif
+ifdef UNSAFE_GETENV
+	COMPAT_CFLAGS += -DUNSAFE_GETENV
+	COMPAT_OBJS += compat/safe-getenv.o
+endif
 ifdef NO_MKDTEMP
 	COMPAT_CFLAGS += -DNO_MKDTEMP
 	COMPAT_OBJS += compat/mkdtemp.o

compat/mingw.h

@@ -198,7 +198,7 @@ char *mingw_getcwd(char *pointer, int len);
 #define getcwd mingw_getcwd
 
 char *mingw_getenv(const char *name);
-#define getenv mingw_getenv
+#define stdlib_getenv mingw_getenv
 
 struct hostent *mingw_gethostbyname(const char *host);
 #define gethostbyname mingw_gethostbyname

compat/safe-getenv.c

@@ -0,0 +1,95 @@
+/*
+ * Fixes git's usage of getenv() on systems with exotic getenv implementations.
+ */
+#include "../git-compat-util.h"
+#include "../cache.h"
+
+#ifdef NO_PTHREADS
+
+#define mutex_init() (void)0
+#define mutex_lock() (void)0
+#define mutex_unlock() (void)0
+
+#else
+
+#include <pthread.h>
+
+static pthread_mutex_t getenv_mutex;
+#define mutex_init() pthread_mutex_init(&getenv_mutex, NULL)
+#define mutex_lock() pthread_mutex_lock(&getenv_mutex)
+#define mutex_unlock() pthread_mutex_unlock(&getenv_mutex)
+
+#endif
+
+#undef getenv
+#ifdef stdlib_getenv
+# define unsafe_getenv stdlib_getenv
+#else
+# define unsafe_getenv getenv
+#endif
+
+/* FNV-1 string hash function, see http://www.isthe.com/chongo/tech/comp/fnv */
+#define FNV32_BASE ((unsigned int) 0x811c9dc5)
+#define FNV32_PRIME ((unsigned int) 0x01000193)
+
+unsigned int strhash(const char *str)
+{
+	unsigned int c, hash = FNV32_BASE;
+	while ((c = (unsigned char) *str++))
+		hash = (hash * FNV32_PRIME) ^ c;
+	return hash;
+}
+
+/*
+ * Entry in a pool of strings that have ever been returned by getenv().
+ */
+struct getenv_pool_entry {
+	struct getenv_pool_entry *next;
+	char value[1]; /* over-allocated to required size */
+};
+
+char *safe_getenv(const char *key)
+{
+	static struct hash_table getenv_pool;
+	static int initialized = 0;
+	int hash;
+	char *value;
+	struct getenv_pool_entry *ent;
+	void **pnext;
+
+	/* lazy initialize mutex + getenv_pool */
+	if (!initialized) {
+		init_hash(&getenv_pool);
+		mutex_init();
+		initialized = 1;
+	}
+
+	/* get value from original getenv */
+	mutex_lock();
+	value = unsafe_getenv(key);
+	if (!value) {
+		mutex_unlock();
+		return NULL;
+	}
+
+	/* lookup and return existing value from pool */
+	hash = strhash(value);
+	ent = lookup_hash(hash, &getenv_pool);
+	while (ent) {
+		if (!strcmp(value, ent->value)) {
+			mutex_unlock();
+			return ent->value;
+		}
+		ent = ent->next;
+	}
+
+	/* not in pool, copy value and insert */
+	ent = xmalloc(sizeof(*ent) + strlen(value));
+	ent->next = NULL;
+	strcpy(ent->value, value);
+	pnext = insert_hash(hash, ent, &getenv_pool);
+	if (pnext)
+		ent->next = *pnext;
+	mutex_unlock();
+	return ent->value;
+}

git-compat-util.h

@@ -321,6 +321,13 @@ extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern int gitsetenv(const char *, const char *, int);
 #endif
 
+#ifdef UNSAFE_GETENV
+extern char *safe_getenv(const char *name);
+# define getenv safe_getenv
+#elif defined(stdlib_getenv)
+# define getenv stdlib_getenv
+#endif
+
 #ifdef NO_MKDTEMP
 #define mkdtemp gitmkdtemp
 extern char *gitmkdtemp(char *);