Skip to content

Commit 0b45507

Browse files
kbleespatthoyts
authored andcommitted
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 <[email protected]>
1 parent 9a5df16 commit 0b45507

File tree

1 file changed

+62
-36
lines changed

1 file changed

+62
-36
lines changed

compat/mingw.c

Lines changed: 62 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -737,23 +737,42 @@ char *mingw_getcwd(char *pointer, int len)
737737
return pointer;
738738
}
739739

740-
static int compareenv(const void *a, const void *b)
740+
/*
741+
* Compare environment entries by key (i.e. stopping at '=' or '\0').
742+
*/
743+
static int compareenv(const void *v1, const void *v2)
741744
{
742-
char *const *ea = a;
743-
char *const *eb = b;
744-
return strcasecmp(*ea, *eb);
745+
const char *e1 = *(const char**)v1;
746+
const char *e2 = *(const char**)v2;
747+
748+
for (;;) {
749+
int c1 = *e1++;
750+
int c2 = *e2++;
751+
c1 = (c1 == '=') ? 0 : tolower(c1);
752+
c2 = (c2 == '=') ? 0 : tolower(c2);
753+
if (c1 > c2)
754+
return 1;
755+
if (c1 < c2)
756+
return -1;
757+
if (c1 == 0)
758+
return 0;
759+
}
745760
}
746761

747-
static int lookupenv(char **env, const char *name, size_t nmln)
762+
static int bsearchenv(char **env, const char *name, size_t size)
748763
{
749-
int i;
750-
751-
for (i = 0; env[i]; i++) {
752-
if (!strncasecmp(env[i], name, nmln) && '=' == env[i][nmln])
753-
/* matches */
754-
return i;
764+
unsigned low = 0, high = size;
765+
while (low < high) {
766+
unsigned mid = low + ((high - low) >> 1);
767+
int cmp = compareenv(&env[mid], &name);
768+
if (cmp < 0)
769+
low = mid + 1;
770+
else if (cmp > 0)
771+
high = mid;
772+
else
773+
return mid;
755774
}
756-
return -1;
775+
return ~low; /* not found, return 1's complement of insert position */
757776
}
758777

759778
/*
@@ -763,26 +782,24 @@ static int lookupenv(char **env, const char *name, size_t nmln)
763782
*/
764783
static int do_putenv(char **env, const char *name, int size, int free_old)
765784
{
766-
char *eq = strchrnul(name, '=');
767-
int i = lookupenv(env, name, eq-name);
785+
int i = bsearchenv(env, name, size - 1);
768786

769-
if (i < 0) {
770-
if (*eq) {
771-
env[size - 1] = (char*) name;
772-
env[size] = NULL;
787+
/* optionally free removed / replaced entry */
788+
if (i >= 0 && free_old)
789+
free(env[i]);
790+
791+
if (strchr(name, '=')) {
792+
/* if new value ('key=value') is specified, insert or replace entry */
793+
if (i < 0) {
794+
i = ~i;
795+
memmove(&env[i + 1], &env[i], (size - i) * sizeof(char*));
773796
size++;
774797
}
775-
}
776-
else {
777-
if (free_old)
778-
free(env[i]);
779-
if (*eq)
780-
env[i] = (char*) name;
781-
else {
782-
for (; env[i]; i++)
783-
env[i] = env[i+1];
784-
size--;
785-
}
798+
env[i] = (char*) name;
799+
} else if (i >= 0) {
800+
/* otherwise ('key') remove existing entry */
801+
size--;
802+
memmove(&env[i], &env[i + 1], (size - i) * sizeof(char*));
786803
}
787804
return size;
788805
}
@@ -792,15 +809,24 @@ static int environ_size = 0;
792809
/* allocated size of environ array, in bytes */
793810
static int environ_alloc = 0;
794811

795-
#undef getenv
812+
static char *do_getenv(const char *name)
813+
{
814+
char *value;
815+
int pos = bsearchenv(environ, name, environ_size - 1);
816+
if (pos < 0)
817+
return NULL;
818+
value = strchr(environ[pos], '=');
819+
return value ? &value[1] : NULL;
820+
}
821+
796822
char *mingw_getenv(const char *name)
797823
{
798-
char *result = getenv(name);
824+
char *result = do_getenv(name);
799825
if (!result && !strcmp(name, "TMPDIR")) {
800826
/* on Windows it is TMP and TEMP */
801-
result = getenv("TMP");
827+
result = do_getenv("TMP");
802828
if (!result)
803-
result = getenv("TEMP");
829+
result = do_getenv("TEMP");
804830
}
805831
else if (!result && !strcmp(name, "TERM")) {
806832
/* simulate TERM to enable auto-color (see color.c) */
@@ -1025,9 +1051,6 @@ static wchar_t *make_environment_block(char **deltaenv)
10251051
for (i = 0; deltaenv && deltaenv[i]; i++)
10261052
size = do_putenv(tmpenv, deltaenv[i], size, 0);
10271053

1028-
/* environment must be sorted */
1029-
qsort(tmpenv, size - 1, sizeof(char*), compareenv);
1030-
10311054
/* create environment block from temporary environment */
10321055
for (i = 0; tmpenv[i]; i++) {
10331056
size = 2 * strlen(tmpenv[i]) + 2; /* +2 for final \0 */
@@ -2107,6 +2130,9 @@ void mingw_startup()
21072130
environ[i] = NULL;
21082131
free(buffer);
21092132

2133+
/* sort environment for O(log n) getenv / putenv */
2134+
qsort(environ, i, sizeof(char*), compareenv);
2135+
21102136
/* initialize critical section for waitpid pinfo_t list */
21112137
InitializeCriticalSection(&pinfo_cs);
21122138

0 commit comments

Comments
 (0)