Win32: keep the environment sorted

The Windows environment is sorted, keep it that way for O(log n) environment access. Change compareenv to compare only the keys, so that it can be used to find an entry irrespective of the value. Change lookupenv to binary seach for an entry. Return one's complement of the insert position if not found (libc's bsearch returns NULL). Replace MSVCRT's getenv with a minimal do_getenv based on the binary search function. Change do_putenv to insert new entries at the correct position. Simplify the function by swapping if conditions and using memmove instead of for loops. Move qsort from make_environment_block to mingw_startup. We still need to sort on startup to make sure that the environment is sorted according to our compareenv function (while Win32 / CreateProcess requires the environment block to be sorted case-insensitively, CreateProcess currently doesn't enforce this, and some applications such as bash just don't care). Note that environment functions are _not_ thread-safe and are not required to be so by POSIX, the application is responsible for synchronizing access to the environment. MSVCRT's getenv and our new getenv implementation are better than that in that they are thread-safe with respect to other getenv calls as long as the environment is not modified. Git's indiscriminate use of getenv in background threads currently requires this property. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Stepan Kasal <kasal@ucw.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Karsten Blees committed Jul 17, 2014 at 17:38 UTC 343ff06da7d83f40892b10a3b653c7d0e6cb526c
1 file changed +65 -39
compat/mingw.c
+65 -39
@@ -906,13 +906,6 @@ static int environ_size = 0;
906 /* allocated size of environ array, in bytes */
907 static int environ_alloc = 0;
908
909 -static int compareenv(const void *a, const void *b)
910 -{
911 - char *const *ea = a;
912 - char *const *eb = b;
913 - return strcasecmp(*ea, *eb);
914 -}
915 -
909 /*
910 * Create environment block suitable for CreateProcess. Merges current
911 * process environment and the supplied environment changes.
@@ -934,9 +927,6 @@ static wchar_t *make_environment_block(char **deltaenv)
927 for (i = 0; deltaenv && deltaenv[i]; i++)
928 size = do_putenv(tmpenv, deltaenv[i], size, 0);
929
937 - /* environment must be sorted */
938 - qsort(tmpenv, size - 1, sizeof(char*), compareenv);
939 -
930 /* create environment block from temporary environment */
931 for (i = 0; tmpenv[i]; i++) {
932 size = 2 * strlen(tmpenv[i]) + 2; /* +2 for final \0 */
@@ -1194,16 +1184,42 @@ int mingw_kill(pid_t pid, int sig)
1184 return -1;
1185 }
1186
1197 -static int lookupenv(char **env, const char *name, size_t nmln)
1198 -{
1199 - int i;
1187 +/*
1188 + * Compare environment entries by key (i.e. stopping at '=' or '\0').
1189 + */
1190 +static int compareenv(const void *v1, const void *v2)
1191 +{
1192 + const char *e1 = *(const char**)v1;
1193 + const char *e2 = *(const char**)v2;
1194 +
1195 + for (;;) {
1196 + int c1 = *e1++;
1197 + int c2 = *e2++;
1198 + c1 = (c1 == '=') ? 0 : tolower(c1);
1199 + c2 = (c2 == '=') ? 0 : tolower(c2);
1200 + if (c1 > c2)
1201 + return 1;
1202 + if (c1 < c2)
1203 + return -1;
1204 + if (c1 == 0)
1205 + return 0;
1206 + }
1207 +}
1208
1201 - for (i = 0; env[i]; i++) {
1202 - if (!strncasecmp(env[i], name, nmln) && '=' == env[i][nmln])
1203 - /* matches */
1204 - return i;
1209 +static int bsearchenv(char **env, const char *name, size_t size)
1210 +{
1211 + unsigned low = 0, high = size;
1212 + while (low < high) {
1213 + unsigned mid = low + ((high - low) >> 1);
1214 + int cmp = compareenv(&env[mid], &name);
1215 + if (cmp < 0)
1216 + low = mid + 1;
1217 + else if (cmp > 0)
1218 + high = mid;
1219 + else
1220 + return mid;
1221 }
1206 - return -1;
1222 + return ~low; /* not found, return 1's complement of insert position */
1223 }
1224
1225 /*
@@ -1213,39 +1229,46 @@ static int lookupenv(char **env, const char *name, size_t nmln)
1229 */
1230 static int do_putenv(char **env, const char *name, int size, int free_old)
1231 {
1216 - char *eq = strchrnul(name, '=');
1217 - int i = lookupenv(env, name, eq-name);
1232 + int i = bsearchenv(env, name, size - 1);
1233
1219 - if (i < 0) {
1220 - if (*eq) {
1221 - env[size - 1] = (char*) name;
1222 - env[size] = NULL;
1234 + /* optionally free removed / replaced entry */
1235 + if (i >= 0 && free_old)
1236 + free(env[i]);
1237 +
1238 + if (strchr(name, '=')) {
1239 + /* if new value ('key=value') is specified, insert or replace entry */
1240 + if (i < 0) {
1241 + i = ~i;
1242 + memmove(&env[i + 1], &env[i], (size - i) * sizeof(char*));
1243 size++;
1244 }
1225 - }
1226 - else {
1227 - if (free_old)
1228 - free(env[i]);
1229 - if (*eq)
1230 - env[i] = (char*) name;
1231 - else {
1232 - for (; env[i]; i++)
1233 - env[i] = env[i+1];
1234 - size--;
1235 - }
1245 + env[i] = (char*) name;
1246 + } else if (i >= 0) {
1247 + /* otherwise ('key') remove existing entry */
1248 + size--;
1249 + memmove(&env[i], &env[i + 1], (size - i) * sizeof(char*));
1250 }
1251 return size;
1252 }
1253
1240 -#undef getenv
1254 +static char *do_getenv(const char *name)
1255 +{
1256 + char *value;
1257 + int pos = bsearchenv(environ, name, environ_size - 1);
1258 + if (pos < 0)
1259 + return NULL;
1260 + value = strchr(environ[pos], '=');
1261 + return value ? &value[1] : NULL;
1262 +}
1263 +
1264 char *mingw_getenv(const char *name)
1265 {
1243 - char *result = getenv(name);
1266 + char *result = do_getenv(name);
1267 if (!result && !strcmp(name, "TMPDIR")) {
1268 /* on Windows it is TMP and TEMP */
1246 - result = getenv("TMP");
1269 + result = do_getenv("TMP");
1270 if (!result)
1248 - result = getenv("TEMP");
1271 + result = do_getenv("TEMP");
1272 }
1273 return result;
1274 }
@@ -2088,6 +2111,9 @@ void mingw_startup()
2111 environ[i] = NULL;
2112 free(buffer);
2113
2114 + /* sort environment for O(log n) getenv / putenv */
2115 + qsort(environ, i, sizeof(char*), compareenv);
2116 +
2117 /* initialize critical section for waitpid pinfo_t list */
2118 InitializeCriticalSection(&pinfo_cs);
2119