Skip to content

Commit dca9d7b

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 4e8b35d commit dca9d7b

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
@@ -974,6 +974,10 @@ static int do_putenv(char **env, const char *name, int size, int free_old);
974974
static int environ_size = 0;
975975
/* allocated size of environ array, in bytes */
976976
static int environ_alloc = 0;
977+
/* used as a indicator when the environment has been changed outside mingw.c */
978+
static char **saved_environ;
979+
980+
static void maybe_reinitialize_environ(void);
977981

978982
/*
979983
* Create environment block suitable for CreateProcess. Merges current
@@ -983,7 +987,10 @@ static wchar_t *make_environment_block(char **deltaenv)
983987
{
984988
wchar_t *wenvblk = NULL;
985989
char **tmpenv;
986-
int i = 0, size = environ_size, wenvsz = 0, wenvpos = 0;
990+
int i = 0, size, wenvsz = 0, wenvpos = 0;
991+
992+
maybe_reinitialize_environ();
993+
size = environ_size;
987994

988995
while (deltaenv && deltaenv[i] && *deltaenv[i])
989996
i++;
@@ -1286,6 +1293,41 @@ static int compareenv(const void *v1, const void *v2)
12861293
}
12871294
}
12881295

1296+
/*
1297+
* Functions implemented outside Git are able to modify the environment,
1298+
* too. For example, cURL's curl_global_init() function sets the CHARSET
1299+
* environment variable (at least in certain circumstances).
1300+
*
1301+
* Therefore we need to be *really* careful *not* to assume that we have
1302+
* sole control over the environment and reinitalize it when necessary.
1303+
*/
1304+
static void maybe_reinitialize_environ(void)
1305+
{
1306+
int i;
1307+
1308+
if (!saved_environ) {
1309+
warning("MinGW environment not initialized yet");
1310+
return;
1311+
}
1312+
1313+
if (environ_size <= 0)
1314+
return;
1315+
1316+
if (saved_environ != environ)
1317+
/* We have *no* idea how much space was allocated outside */
1318+
environ_alloc = 0;
1319+
else if (!environ[environ_size - 1])
1320+
return; /* still consistent */
1321+
1322+
for (i = 0; environ[i] && *environ[i]; i++)
1323+
; /* continue counting */
1324+
environ[i] = NULL;
1325+
environ_size = i + 1;
1326+
1327+
/* sort environment for O(log n) getenv / putenv */
1328+
qsort(environ, i, sizeof(char*), compareenv);
1329+
}
1330+
12891331
static int bsearchenv(char **env, const char *name, size_t size)
12901332
{
12911333
unsigned low = 0, high = size;
@@ -1339,6 +1381,7 @@ char *mingw_getenv(const char *name)
13391381
if (environ_size <= 0)
13401382
return NULL;
13411383

1384+
maybe_reinitialize_environ();
13421385
pos = bsearchenv(environ, name, environ_size - 1);
13431386

13441387
if (pos < 0)
@@ -1349,7 +1392,9 @@ char *mingw_getenv(const char *name)
13491392

13501393
int mingw_putenv(const char *namevalue)
13511394
{
1395+
maybe_reinitialize_environ();
13521396
ALLOC_GROW(environ, (environ_size + 1) * sizeof(char*), environ_alloc);
1397+
saved_environ = environ;
13531398
environ_size = do_putenv(environ, namevalue, environ_size, 1);
13541399
return 0;
13551400
}
@@ -2237,7 +2282,7 @@ void mingw_startup()
22372282
*/
22382283
environ_size = i + 1;
22392284
environ_alloc = alloc_nr(environ_size * sizeof(char*));
2240-
environ = malloc_startup(environ_alloc);
2285+
saved_environ = environ = malloc_startup(environ_alloc);
22412286

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

0 commit comments

Comments
 (0)