Skip to content

Commit a0c1a86

Browse files
committed
mingw: be *very* wary about outside environment changes
The environment is modified in most surprising circumstances, and not all of them are under Git's control. For example, calling curl_global_init() on Windows will ensure that the CHARSET variable is set, adding one if necessary. While the previous commit worked around crashes triggered by such outside changes of the environment by relaxing the requirement that the environment be terminated by a NULL pointer, the other assumption made by `mingw_getenv()` and `mingw_putenv()` is that the environment is sorted, for efficient lookup via binary search. Let's make real sure that our environment is intact before querying or modifying it, and reinitialize our idea of the environment if necessary. With this commit, before working on the environment we look briefly for indicators that the environment was modified outside of our control, and to ensure that it is terminated with a NULL pointer and sorted again in that case. Note: the indicators are maybe not sufficient. For example, when a variable is removed, it will not be noticed. It might also be a problem if outside changes to the environment result in a modified `environ` pointer: it is unclear whether such a modification could result in a problem when `mingw_putenv()` needs to `realloc()` the environment buffer. For the moment, however, the current fix works well enough, so let's only face the potential problems when (and if!) they occur. Signed-off-by: Johannes Schindelin <[email protected]>
1 parent adc23c1 commit a0c1a86

File tree

1 file changed

+47
-2
lines changed

1 file changed

+47
-2
lines changed

compat/mingw.c

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,10 @@ static int do_putenv(char **env, const char *name, int size, int free_old);
10301030
static int environ_size = 0;
10311031
/* allocated size of environ array, in bytes */
10321032
static int environ_alloc = 0;
1033+
/* used as a indicator when the environment has been changed outside mingw.c */
1034+
static char **saved_environ;
1035+
1036+
static void maybe_reinitialize_environ(void);
10331037

10341038
/*
10351039
* Create environment block suitable for CreateProcess. Merges current
@@ -1039,7 +1043,10 @@ static wchar_t *make_environment_block(char **deltaenv)
10391043
{
10401044
wchar_t *wenvblk = NULL;
10411045
char **tmpenv;
1042-
int i = 0, size = environ_size, wenvsz = 0, wenvpos = 0;
1046+
int i = 0, size, wenvsz = 0, wenvpos = 0;
1047+
1048+
maybe_reinitialize_environ();
1049+
size = environ_size;
10431050

10441051
while (deltaenv && deltaenv[i] && *deltaenv[i])
10451052
i++;
@@ -1342,6 +1349,41 @@ static int compareenv(const void *v1, const void *v2)
13421349
}
13431350
}
13441351

1352+
/*
1353+
* Functions implemented outside Git are able to modify the environment,
1354+
* too. For example, cURL's curl_global_init() function sets the CHARSET
1355+
* environment variable (at least in certain circumstances).
1356+
*
1357+
* Therefore we need to be *really* careful *not* to assume that we have
1358+
* sole control over the environment and reinitalize it when necessary.
1359+
*/
1360+
static void maybe_reinitialize_environ(void)
1361+
{
1362+
int i;
1363+
1364+
if (!saved_environ) {
1365+
warning("MinGW environment not initialized yet");
1366+
return;
1367+
}
1368+
1369+
if (environ_size <= 0)
1370+
return;
1371+
1372+
if (saved_environ != environ)
1373+
/* We have *no* idea how much space was allocated outside */
1374+
environ_alloc = 0;
1375+
else if (!environ[environ_size - 1])
1376+
return; /* still consistent */
1377+
1378+
for (i = 0; environ[i] && *environ[i]; i++)
1379+
; /* continue counting */
1380+
environ[i] = NULL;
1381+
environ_size = i + 1;
1382+
1383+
/* sort environment for O(log n) getenv / putenv */
1384+
qsort(environ, i, sizeof(char*), compareenv);
1385+
}
1386+
13451387
static int bsearchenv(char **env, const char *name, size_t size)
13461388
{
13471389
unsigned low = 0, high = size;
@@ -1395,6 +1437,7 @@ char *mingw_getenv(const char *name)
13951437
if (environ_size <= 0)
13961438
return NULL;
13971439

1440+
maybe_reinitialize_environ();
13981441
pos = bsearchenv(environ, name, environ_size - 1);
13991442

14001443
if (pos < 0)
@@ -1405,7 +1448,9 @@ char *mingw_getenv(const char *name)
14051448

14061449
int mingw_putenv(const char *namevalue)
14071450
{
1451+
maybe_reinitialize_environ();
14081452
ALLOC_GROW(environ, (environ_size + 1) * sizeof(char*), environ_alloc);
1453+
saved_environ = environ;
14091454
environ_size = do_putenv(environ, namevalue, environ_size, 1);
14101455
return 0;
14111456
}
@@ -2326,7 +2371,7 @@ void mingw_startup()
23262371
*/
23272372
environ_size = i + 1;
23282373
environ_alloc = alloc_nr(environ_size * sizeof(char*));
2329-
environ = malloc_startup(environ_alloc);
2374+
saved_environ = environ = malloc_startup(environ_alloc);
23302375

23312376
/* allocate buffer (wchar_t encodes to max 3 UTF-8 bytes) */
23322377
maxlen = 3 * maxlen + 1;

0 commit comments

Comments
 (0)