* [PATCH 1/2] overhaul environment functions
2017-09-03 19:12 [PATCH 0/2] env functions overhaul Alexander Monakov
@ 2017-09-03 19:12 ` Alexander Monakov
2017-09-03 19:12 ` [PATCH 2/2] free allocations in clearenv Alexander Monakov
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Monakov @ 2017-09-03 19:12 UTC (permalink / raw)
To: musl
Rewrite environment access functions to slim down code, fix bugs and
avoid invoking undefined behavior.
* avoid using int-typed iterators where size_t would be correct;
* use strncmp instead of memcmp consistently;
* tighten prologues by invoking __strchrnul;
* handle NULL environ.
putenv:
* handle "=value" input via unsetenv too (will return -1/EINVAL);
* rewrite and simplify __putenv; fix the leak caused by failure to
deallocate entry added by preceding setenv when called from putenv.
setenv:
* move management of libc-allocated entries to this translation unit,
and use no-op weak symbols in putenv/unsetenv;
unsetenv:
* rewrite; this fixes UB caused by testing a free'd pointer against
NULL on entry to subsequent loops.
Not changed:
Failure to extend allocation tracking array (previously __env_map, now
env_alloced) is ignored rather than causing to report -1/ENOMEM to the
caller; the worst-case consequence is leaking this allocation when it
is removed or replaced in a subsequent environment access.
Initially UB in unsetenv was reported by Alexander Cherepanov.
Using a weak alias to avoid pulling in malloc via unsetenv was
suggested by Rich Felker.
---
src/env/getenv.c | 15 ++++----
src/env/putenv.c | 106 ++++++++++++++++++++++++-----------------------------
src/env/setenv.c | 41 ++++++++++++++-------
src/env/unsetenv.c | 61 +++++++++++++++---------------
4 files changed, 114 insertions(+), 109 deletions(-)
rewrite src/env/putenv.c (88%)
rewrite src/env/unsetenv.c (77%)
diff --git a/src/env/getenv.c b/src/env/getenv.c
index 00c1bce0..cf34672c 100644
--- a/src/env/getenv.c
+++ b/src/env/getenv.c
@@ -2,13 +2,14 @@
#include <string.h>
#include "libc.h"
+char *__strchrnul(const char *, int);
+
char *getenv(const char *name)
{
- int i;
- size_t l = strlen(name);
- if (!__environ || !*name || strchr(name, '=')) return NULL;
- for (i=0; __environ[i] && (strncmp(name, __environ[i], l)
- || __environ[i][l] != '='); i++);
- if (__environ[i]) return __environ[i] + l+1;
- return NULL;
+ size_t l = __strchrnul(name, '=') - name;
+ if (l && !name[l] && __environ)
+ for (char **e = __environ; *e; e++)
+ if (!strncmp(name, *e, l) && l[*e] == '=')
+ return *e + l+1;
+ return 0;
}
diff --git a/src/env/putenv.c b/src/env/putenv.c
dissimilarity index 88%
index 71530426..fa4a4ddc 100644
--- a/src/env/putenv.c
+++ b/src/env/putenv.c
@@ -1,58 +1,48 @@
-#include <stdlib.h>
-#include <string.h>
-
-extern char **__environ;
-char **__env_map;
-
-int __putenv(char *s, int a)
-{
- int i=0, j=0;
- char *z = strchr(s, '=');
- char **newenv = 0;
- char **newmap = 0;
- static char **oldenv;
-
- if (!z) return unsetenv(s);
- if (z==s) return -1;
- for (; __environ[i] && memcmp(s, __environ[i], z-s+1); i++);
- if (a) {
- if (!__env_map) {
- __env_map = calloc(2, sizeof(char *));
- if (__env_map) __env_map[0] = s;
- } else {
- for (; __env_map[j] && __env_map[j] != __environ[i]; j++);
- if (!__env_map[j]) {
- newmap = realloc(__env_map, sizeof(char *)*(j+2));
- if (newmap) {
- __env_map = newmap;
- __env_map[j] = s;
- __env_map[j+1] = NULL;
- }
- } else {
- free(__env_map[j]);
- __env_map[j] = s;
- }
- }
- }
- if (!__environ[i]) {
- newenv = malloc(sizeof(char *)*(i+2));
- if (!newenv) {
- if (a && __env_map) __env_map[j] = 0;
- return -1;
- }
- memcpy(newenv, __environ, sizeof(char *)*i);
- newenv[i] = s;
- newenv[i+1] = 0;
- __environ = newenv;
- free(oldenv);
- oldenv = __environ;
- }
-
- __environ[i] = s;
- return 0;
-}
-
-int putenv(char *s)
-{
- return __putenv(s, 0);
-}
+#include <stdlib.h>
+#include <string.h>
+#include "libc.h"
+
+char *__strchrnul(const char *, int);
+
+static void dummy(char *old, char *new) {}
+weak_alias(dummy, __env_rm_add);
+
+int __putenv(char *s, size_t l, char *r)
+{
+ size_t i=0;
+ if (__environ) {
+ for (char **e = __environ; *e; e++, i++)
+ if (!strncmp(s, *e, l+1)) {
+ char *tmp = *e;
+ *e = s;
+ __env_rm_add(tmp, r);
+ return 0;
+ }
+ }
+ static char **oldenv;
+ char **newenv;
+ if (__environ == oldenv) {
+ newenv = realloc(oldenv, sizeof *newenv * (i+2));
+ if (!newenv) goto oom;
+ } else {
+ newenv = malloc(sizeof *newenv * (i+2));
+ if (!newenv) goto oom;
+ if (i) memcpy(newenv, __environ, sizeof *newenv * i);
+ free(oldenv);
+ }
+ newenv[i] = s;
+ newenv[i+1] = 0;
+ __environ = oldenv = newenv;
+ if (r) __env_rm_add(0, r);
+ return 0;
+oom:
+ free(r);
+ return -1;
+}
+
+int putenv(char *s)
+{
+ size_t l = __strchrnul(s, '=') - s;
+ if (!l || !s[l]) return unsetenv(s);
+ return __putenv(s, l, 0);
+}
diff --git a/src/env/setenv.c b/src/env/setenv.c
index 76e8ee12..a7dd2b60 100644
--- a/src/env/setenv.c
+++ b/src/env/setenv.c
@@ -2,29 +2,44 @@
#include <string.h>
#include <errno.h>
-int __putenv(char *s, int a);
+char *__strchrnul(const char *, int);
+int __putenv(char *, size_t, char *);
+
+void __env_rm_add(char *old, char *new)
+{
+ static char **env_alloced;
+ static size_t env_alloced_n;
+ for (size_t i=0; i < env_alloced_n; i++)
+ if (env_alloced[i] == old) {
+ env_alloced[i] = new;
+ free(old);
+ return;
+ } else if (!env_alloced[i] && new) {
+ env_alloced[i] = new;
+ new = 0;
+ }
+ if (!new) return;
+ char **t = realloc(env_alloced, sizeof *t * (env_alloced_n+1));
+ if (!t) return;
+ (env_alloced = t)[env_alloced_n++] = new;
+}
int setenv(const char *var, const char *value, int overwrite)
{
char *s;
- int l1, l2;
+ size_t l1, l2;
- if (!var || !*var || strchr(var, '=')) {
+ if (!var || !(l1 = __strchrnul(var, '=') - var) || var[l1]) {
errno = EINVAL;
return -1;
}
if (!overwrite && getenv(var)) return 0;
- l1 = strlen(var);
l2 = strlen(value);
s = malloc(l1+l2+2);
- if (s) {
- memcpy(s, var, l1);
- s[l1] = '=';
- memcpy(s+l1+1, value, l2);
- s[l1+l2+1] = 0;
- if (!__putenv(s, 1)) return 0;
- }
- free(s);
- return -1;
+ if (!s) return -1;
+ memcpy(s, var, l1);
+ s[l1] = '=';
+ memcpy(s+l1+1, value, l2+1);
+ return __putenv(s, l1, s);
}
diff --git a/src/env/unsetenv.c b/src/env/unsetenv.c
dissimilarity index 77%
index 35693354..8630e2d7 100644
--- a/src/env/unsetenv.c
+++ b/src/env/unsetenv.c
@@ -1,31 +1,30 @@
-#include <stdlib.h>
-#include <string.h>
-#include <errno.h>
-
-extern char **__environ;
-extern char **__env_map;
-
-int unsetenv(const char *name)
-{
- int i, j;
- size_t l = strlen(name);
-
- if (!*name || strchr(name, '=')) {
- errno = EINVAL;
- return -1;
- }
-again:
- for (i=0; __environ[i] && (memcmp(name, __environ[i], l) || __environ[i][l] != '='); i++);
- if (__environ[i]) {
- if (__env_map) {
- for (j=0; __env_map[j] && __env_map[j] != __environ[i]; j++);
- free (__env_map[j]);
- for (; __env_map[j]; j++)
- __env_map[j] = __env_map[j+1];
- }
- for (; __environ[i]; i++)
- __environ[i] = __environ[i+1];
- goto again;
- }
- return 0;
-}
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include "libc.h"
+
+char *__strchrnul(const char *, int);
+
+static void dummy(char *old, char *new) {}
+weak_alias(dummy, __env_rm_add);
+
+int unsetenv(const char *name)
+{
+ size_t l = __strchrnul(name, '=') - name;
+ if (!l || name[l]) {
+ errno = EINVAL;
+ return -1;
+ }
+ if (__environ) {
+ char **e = __environ, **eo = e;
+ for (; *e; e++)
+ if (!strncmp(name, *e, l) && l[*e] == '=')
+ __env_rm_add(*e, 0);
+ else if (eo != e)
+ *eo++ = *e;
+ else
+ eo++;
+ if (eo != e) *eo = 0;
+ }
+ return 0;
+}
--
2.11.0
^ permalink raw reply [flat|nested] 3+ messages in thread