Skip to content

Commit f48707f

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 3a51fd0 commit f48707f

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
@@ -957,6 +957,10 @@ static int do_putenv(char **env, const char *name, int size, int free_old);
957957
static int environ_size = 0;
958958
/* allocated size of environ array, in bytes */
959959
static int environ_alloc = 0;
960+
/* used as a indicator when the environment has been changed outside mingw.c */
961+
static char **saved_environ;
962+
963+
static void maybe_reinitialize_environ(void);
960964

961965
/*
962966
* Create environment block suitable for CreateProcess. Merges current
@@ -966,7 +970,10 @@ static wchar_t *make_environment_block(char **deltaenv)
966970
{
967971
wchar_t *wenvblk = NULL;
968972
char **tmpenv;
969-
int i = 0, size = environ_size, wenvsz = 0, wenvpos = 0;
973+
int i = 0, size, wenvsz = 0, wenvpos = 0;
974+
975+
maybe_reinitialize_environ();
976+
size = environ_size;
970977

971978
while (deltaenv && deltaenv[i] && *deltaenv[i])
972979
i++;
@@ -1269,6 +1276,41 @@ static int compareenv(const void *v1, const void *v2)
12691276
}
12701277
}
12711278

1279+
/*
1280+
* Functions implemented outside Git are able to modify the environment,
1281+
* too. For example, cURL's curl_global_init() function sets the CHARSET
1282+
* environment variable (at least in certain circumstances).
1283+
*
1284+
* Therefore we need to be *really* careful *not* to assume that we have
1285+
* sole control over the environment and reinitalize it when necessary.
1286+
*/
1287+
static void maybe_reinitialize_environ(void)
1288+
{
1289+
int i;
1290+
1291+
if (!saved_environ) {
1292+
warning("MinGW environment not initialized yet");
1293+
return;
1294+
}
1295+
1296+
if (environ_size <= 0)
1297+
return;
1298+
1299+
if (saved_environ != environ)
1300+
/* We have *no* idea how much space was allocated outside */
1301+
environ_alloc = 0;
1302+
else if (!environ[environ_size - 1])
1303+
return; /* still consistent */
1304+
1305+
for (i = 0; environ[i] && *environ[i]; i++)
1306+
; /* continue counting */
1307+
environ[i] = NULL;
1308+
environ_size = i + 1;
1309+
1310+
/* sort environment for O(log n) getenv / putenv */
1311+
qsort(environ, i, sizeof(char*), compareenv);
1312+
}
1313+
12721314
static int bsearchenv(char **env, const char *name, size_t size)
12731315
{
12741316
unsigned low = 0, high = size;
@@ -1322,6 +1364,7 @@ char *mingw_getenv(const char *name)
13221364
if (environ_size <= 0)
13231365
return NULL;
13241366

1367+
maybe_reinitialize_environ();
13251368
pos = bsearchenv(environ, name, environ_size - 1);
13261369

13271370
if (pos < 0)
@@ -1332,7 +1375,9 @@ char *mingw_getenv(const char *name)
13321375

13331376
int mingw_putenv(const char *namevalue)
13341377
{
1378+
maybe_reinitialize_environ();
13351379
ALLOC_GROW(environ, (environ_size + 1) * sizeof(char*), environ_alloc);
1380+
saved_environ = environ;
13361381
environ_size = do_putenv(environ, namevalue, environ_size, 1);
13371382
return 0;
13381383
}
@@ -2220,7 +2265,7 @@ void mingw_startup()
22202265
*/
22212266
environ_size = i + 1;
22222267
environ_alloc = alloc_nr(environ_size * sizeof(char*));
2223-
environ = malloc_startup(environ_alloc);
2268+
saved_environ = environ = malloc_startup(environ_alloc);
22242269

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

0 commit comments

Comments
 (0)