Hello,
the current implementation of setenv(3), putenv(3), unsetenv(3) and
getenv(3) is a bit messy. This mostly because we try hard to keep
the environment array and the array with the allocat memory
block in sync.
I've attached a new implementation which remembers the memory blocks
in a seperate tree as suggested by Enami Tsugutomo. The code passes
the ATF regression tests including "t_getenv" which currently fails
as our implementation of getenv(3) is slightly broken.
I would appreciate a code review and other comments.
Kind regards
--
Matthias Scheler http://zhadum.org.uk/
Index: lib/libc/compat/stdlib/compat_unsetenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/compat/stdlib/compat_unsetenv.c,v
retrieving revision 1.2
diff -u -r1.2 compat_unsetenv.c
--- lib/libc/compat/stdlib/compat_unsetenv.c 23 Sep 2010 17:30:49 -0000
1.2
+++ lib/libc/compat/stdlib/compat_unsetenv.c 13 Nov 2010 21:32:44 -0000
@@ -54,16 +54,12 @@
__weak_alias(unsetenv,_unsetenv)
#endif
-#ifdef _REENTRANT
-extern rwlock_t __environ_lock;
-#endif
+#include "env.h"
__warn_references(unsetenv,
"warning: reference to compatibility unsetenv();"
" include <stdlib.h> for correct reference")
-extern char **environ;
-
/*
* unsetenv(name) --
* Delete environmental variable "name".
@@ -71,15 +67,21 @@
void
unsetenv(const char *name)
{
- char **p;
- int offset;
+ size_t l_name;
_DIAGASSERT(name != NULL);
- rwlock_wrlock(&__environ_lock);
- while (__findenv(name, &offset)) /* if set multiple times */
- for (p = &environ[offset];; ++p)
- if (!(*p = *(p + 1)))
- break;
- rwlock_unlock(&__environ_lock);
+ l_name = strlen(name);
+ if (__writelockenv()) {
+ ssize_t offset;
+
+ while ((offset = __getenvslot(name, l_name, false)) != -1) {
+ char **p;
+ for (p = &environ[offset]; ; ++p) {
+ if ((*p = *(p + 1)) == NULL)
+ break;
+ }
+ }
+ (void)__unlockenv();
+ }
}
Index: lib/libc/gen/popen.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/popen.c,v
retrieving revision 1.29
diff -u -r1.29 popen.c
--- lib/libc/gen/popen.c 15 Oct 2006 16:12:02 -0000 1.29
+++ lib/libc/gen/popen.c 13 Nov 2010 21:32:44 -0000
@@ -54,6 +54,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+
+#include "env.h"
#include "reentrant.h"
#ifdef __weak_alias
@@ -61,10 +63,6 @@
__weak_alias(pclose,_pclose)
#endif
-#ifdef _REENTRANT
-extern rwlock_t __environ_lock;
-#endif
-
static struct pid {
struct pid *next;
FILE *fp;
@@ -111,13 +109,13 @@
return (NULL);
}
- rwlock_rdlock(&pidlist_lock);
- rwlock_rdlock(&__environ_lock);
+ (void)rwlock_rdlock(&pidlist_lock);
+ (void)__readlockenv();
switch (pid = vfork()) {
case -1: /* Error. */
serrno = errno;
- rwlock_unlock(&__environ_lock);
- rwlock_unlock(&pidlist_lock);
+ (void)__unlockenv();
+ (void)rwlock_unlock(&pidlist_lock);
free(cur);
(void)close(pdes[0]);
(void)close(pdes[1]);
@@ -155,7 +153,7 @@
_exit(127);
/* NOTREACHED */
}
- rwlock_unlock(&__environ_lock);
+ (void)__unlockenv();
/* Parent; assume fdopen can't fail. */
if (*xtype == 'r') {
@@ -177,7 +175,7 @@
cur->pid = pid;
cur->next = pidlist;
pidlist = cur;
- rwlock_unlock(&pidlist_lock);
+ (void)rwlock_unlock(&pidlist_lock);
return (iop);
}
@@ -204,7 +202,7 @@
if (cur->fp == iop)
break;
if (cur == NULL) {
- rwlock_unlock(&pidlist_lock);
+ (void)rwlock_unlock(&pidlist_lock);
return (-1);
}
@@ -216,7 +214,7 @@
else
last->next = cur->next;
- rwlock_unlock(&pidlist_lock);
+ (void)rwlock_unlock(&pidlist_lock);
do {
pid = waitpid(cur->pid, &pstat, 0);
Index: lib/libc/include/env.h
===================================================================
RCS file: lib/libc/include/env.h
diff -N lib/libc/include/env.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ lib/libc/include/env.h 13 Nov 2010 21:32:44 -0000
@@ -0,0 +1,65 @@
+/* $NetBSD$ */
+
+/*-
+ * Copyright (c) 2010 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Matthias Scheler.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/types.h>
+
+#include <stdbool.h>
+
+#include "reentrant.h"
+
+extern ssize_t __getenvslot(const char *name, size_t l_name, bool allocate);
+extern char *__findenv(const char *name, size_t l_name);
+
+#ifdef _REENTRANT
+extern bool __readlockenv(void);
+extern bool __writelockenv(void);
+extern bool __unlockenv(void);
+#else
+static __inline bool
+__readlockenv(void)
+{
+ return true;
+}
+
+static __inline bool
+__writelockenv(void)
+{
+ return true;
+}
+
+static __inline bool
+__unlocklockenv(void)
+{
+ return true;
+}
+#endif
+
+extern char **environ;
Index: lib/libc/misc/initfini.c
===================================================================
RCS file: /cvsroot/src/lib/libc/misc/initfini.c,v
retrieving revision 1.6
diff -u -r1.6 initfini.c
--- lib/libc/misc/initfini.c 28 Jun 2010 21:58:02 -0000 1.6
+++ lib/libc/misc/initfini.c 13 Nov 2010 21:32:44 -0000
@@ -42,6 +42,7 @@
void __libc_thr_init(void);
void __libc_atomic_init(void);
void __libc_atexit_init(void);
+void __libc_env_init(void);
/* LINTED used */
void
@@ -59,4 +60,7 @@
/* Initialize the atexit mutexes */
__libc_atexit_init();
+
+ /* Initialize environment memory RB tree. */
+ __libc_env_init();
}
Index: lib/libc/stdlib/Makefile.inc
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/Makefile.inc,v
retrieving revision 1.74
diff -u -r1.74 Makefile.inc
--- lib/libc/stdlib/Makefile.inc 3 May 2010 05:01:53 -0000 1.74
+++ lib/libc/stdlib/Makefile.inc 13 Nov 2010 21:32:44 -0000
@@ -4,7 +4,7 @@
# stdlib sources
.PATH: ${ARCHDIR}/stdlib ${.CURDIR}/stdlib
-SRCS+= _rand48.c \
+SRCS+= _env.c _rand48.c \
a64l.c abort.c atexit.c atof.c atoi.c atol.c atoll.c \
bsearch.c drand48.c exit.c \
getenv.c getopt.c getopt_long.c getsubopt.c \
Index: lib/libc/stdlib/_env.c
===================================================================
RCS file: lib/libc/stdlib/_env.c
diff -N lib/libc/stdlib/_env.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ lib/libc/stdlib/_env.c 13 Nov 2010 21:32:44 -0000
@@ -0,0 +1,328 @@
+/* $NetBSD$ */
+
+/*-
+ * Copyright (c) 2010 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Matthias Scheler.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/rbtree.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <string.h>
+
+#include "env.h"
+#include "reentrant.h"
+#include "local.h"
+
+/*
+ * Red-Black tree node for tracking memory used by environment variables.
+ * The tree is sorted by the address of the nodes themselves.
+ */
+typedef struct env_node_s {
+ rb_node_t rb_node;
+ size_t length;
+ char data[0];
+} env_node_t;
+
+/* Compare functions for above tree. */
+static signed int env_tree_compare_nodes(void *, const void *, const void *);
+static signed int env_tree_compare_key(void *, const void *, const void *);
+
+/* Operations for above tree. */
+static const rb_tree_ops_t env_tree_ops = {
+ .rbto_compare_nodes = env_tree_compare_nodes,
+ .rbto_compare_key = env_tree_compare_key,
+ .rbto_node_offset = offsetof(env_node_t, rb_node),
+ .rbto_context = NULL
+};
+
+/* The single instance of above tree. */
+static rb_tree_t env_tree;
+
+/* The allocated environment. */
+static char **allocated_environ;
+static size_t allocated_environ_size;
+
+#define ENV_ARRAY_SIZE_MIN 16
+
+/* The lock protecting accces to the environment. */
+#ifdef _REENTRANT
+static rwlock_t env_lock = RWLOCK_INITIALIZER;
+#endif
+
+/* Our initialization function. */
+void __libc_env_init(void);
+
+/*ARGSUSED*/
+static signed int
+env_tree_compare_nodes(void *ctx, const void *node_a, const void *node_b)
+{
+ uintptr_t addr_a, addr_b;
+
+ addr_a = (uintptr_t)node_a;
+ addr_b = (uintptr_t)node_b;
+
+ if (addr_a < addr_b)
+ return -1;
+
+ if (addr_a > addr_b)
+ return 1;
+
+ return 0;
+}
+
+static signed int
+env_tree_compare_key(void *ctx, const void *node, const void *key)
+{
+ return env_tree_compare_nodes(ctx, node,
+ (const uint8_t *)key - offsetof(env_node_t, data));
+}
+
+/*
+ * Determine the of the name in an environment string. Return 0 if the
+ * name is not valid.
+ */
+size_t
+__envvarnamelen(const char *str, bool withequal)
+{
+ size_t l_name;
+
+ if (str == NULL)
+ return 0;
+
+ l_name = strcspn(str, "=");
+ if (l_name == 0)
+ return 0;
+
+ if (withequal) {
+ if (str[l_name] != '=')
+ return 0;
+ } else {
+ if (str[l_name] == '=')
+ return 0;
+ }
+
+ return l_name;
+}
+
+/*
+ * Free memory occupied by environment variable if possible. This function
+ * must be called with the environment write locked.
+ */
+void
+__freeenvvar(char *envvar)
+{
+ env_node_t *node;
+
+ _DIAGASSERT(envvar != NULL);
+ node = rb_tree_find_node(&env_tree, envvar);
+ if (node != NULL) {
+ rb_tree_remove_node(&env_tree, node);
+ free(node);
+ }
+}
+
+/*
+ * Allocate memory for an environment variable. This function must be called
+ * with the environment write locked.
+ */
+char *
+__allocenvvar(size_t length)
+{
+ env_node_t *node;
+
+ node = malloc(sizeof(env_node_t) + length);
+ if (node != NULL) {
+ node->length = length;
+ rb_tree_insert_node(&env_tree, node);
+ return &node->data[0];
+ } else {
+ return NULL;
+ }
+}
+
+/*
+ * Check whether an enviroment variable is writable. This function must be
+ * called with the environment write locked as the caller will probably
+ * overwrite the enviroment variable afterwards.
+ */
+bool
+__canoverwriteenvvar(char *envvar, size_t length)
+{
+ env_node_t *node;
+
+ _DIAGASSERT(envvar != NULL);
+
+ node = rb_tree_find_node(&env_tree, envvar);
+ return (node != NULL && length <= node->length);
+}
+
+/*
+ * Get a (new) slot in the environment. This function must be called with
+ * the environment write locked.
+ */
+ssize_t
+__getenvslot(const char *name, size_t l_name, bool allocate)
+{
+ size_t new_size, num_entries, required_size;
+ char **new_environ;
+
+ /* Search for an existing environment variable of the given name. */
+ num_entries = 0;
+ while (environ[num_entries] != NULL) {
+ if (strncmp(environ[num_entries], name, l_name) == 0 &&
+ environ[num_entries][l_name] == '=') {
+ /* We found a match. */
+ return num_entries;
+ }
+ num_entries ++;
+ }
+
+ /* No match found, return if we don't want to allocate a new slot. */
+ if (!allocate)
+ return -1;
+
+ /* Create a new slot in the environment. */
+ required_size = num_entries + 1;
+ if (environ == allocated_environ &&
+ required_size < allocated_environ_size) {
+ size_t offset;
+
+ /*
+ * Scrub the environment if somebody erased its contents by
+ * e.g. setting environ[0] to NULL.
+ */
+ offset = required_size;
+ while (offset < allocated_environ_size &&
+ environ[offset] != NULL) {
+ __freeenvvar(environ[offset]);
+ environ[offset] = NULL;
+ offset++;
+ }
+
+ /* Return a free slot. */
+ return num_entries;
+ }
+
+ /* Determine size of a new environment array. */
+ new_size = ENV_ARRAY_SIZE_MIN;
+ while (new_size <= required_size)
+ new_size <<= 1;
+
+ /* Allocate a new environment array. */
+ if (environ == allocated_environ) {
+ new_environ = realloc(environ, new_size * sizeof(char *));
+ if (new_environ == NULL)
+ return -1;
+ } else {
+ free(allocated_environ);
+ allocated_environ = NULL;
+
+ new_environ = malloc(new_size * sizeof(char *));
+ if (new_environ == NULL)
+ return -1;
+ (void)memcpy(new_environ, environ,
+ num_entries * sizeof(char *));
+ }
+
+ /* Clear remaining entries. */
+ (void)memset(&new_environ[num_entries], 0,
+ (new_size - num_entries) * sizeof(char *));
+
+ /* Use the new environent array. */
+ environ = allocated_environ = new_environ;
+ allocated_environ_size = new_size;
+
+ /* Return a free slot. */
+ return num_entries;
+}
+
+/* Find a string in the environment. */
+char *
+__findenv(const char *name, size_t l_name)
+{
+ ssize_t offset;
+
+ offset = __getenvslot(name, l_name, false);
+ return (offset != -1) ? environ[offset] + l_name + 1 : NULL;
+}
+
+#ifdef _REENTRANT
+
+/* Lock the environment for read. */
+bool
+__readlockenv(void)
+{
+ int error;
+
+ error = rwlock_rdlock(&env_lock);
+ if (error == 0)
+ return true;
+
+ errno = error;
+ return false;
+}
+
+/* Lock the environment for write. */
+bool
+__writelockenv(void)
+{
+ int error;
+
+ error = rwlock_wrlock(&env_lock);
+ if (error == 0)
+ return true;
+
+ errno = error;
+ return false;
+}
+
+/* Unlock the environment for write. */
+bool
+__unlockenv(void)
+{
+ int error;
+
+ error = rwlock_unlock(&env_lock);
+ if (error == 0)
+ return true;
+
+ errno = error;
+ return false;
+}
+
+#endif
+
+/* Initialize environment memory RB tree. */
+void
+__libc_env_init(void)
+{
+ rb_tree_init(&env_tree, &env_tree_ops);
+}
Index: lib/libc/stdlib/getenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/getenv.c,v
retrieving revision 1.32
diff -u -r1.32 getenv.c
--- lib/libc/stdlib/getenv.c 10 Nov 2010 02:40:08 -0000 1.32
+++ lib/libc/stdlib/getenv.c 13 Nov 2010 21:32:44 -0000
@@ -43,16 +43,11 @@
#include <errno.h>
#include <stdlib.h>
#include <string.h>
+
+#include "env.h"
#include "reentrant.h"
#include "local.h"
-#ifdef _REENTRANT
-rwlock_t __environ_lock = RWLOCK_INITIALIZER;
-#endif
-char **__environ_malloced;
-static char **saveenv;
-static size_t environ_malloced_len;
-
__weak_alias(getenv_r, _getenv_r)
/*
@@ -65,139 +60,52 @@
char *
getenv(const char *name)
{
- int offset;
+ size_t l_name;
char *result;
_DIAGASSERT(name != NULL);
- rwlock_rdlock(&__environ_lock);
- result = __findenv(name, &offset);
- rwlock_unlock(&__environ_lock);
+ l_name = __envvarnamelen(name, false);
+ if (l_name == 0)
+ return NULL;
+
+ result = NULL;
+ if (__readlockenv()) {
+ result = __findenv(name, l_name);
+ (void)__unlockenv();
+ }
+
return result;
}
int
getenv_r(const char *name, char *buf, size_t len)
{
- int offset;
- char *result;
- int rv = -1;
+ size_t l_name;
+ int rv;
_DIAGASSERT(name != NULL);
- rwlock_rdlock(&__environ_lock);
- result = __findenv(name, &offset);
- if (result == NULL) {
- errno = ENOENT;
- goto out;
- }
- if (strlcpy(buf, result, len) >= len) {
- errno = ERANGE;
- goto out;
- }
- rv = 0;
-out:
- rwlock_unlock(&__environ_lock);
- return rv;
-}
-
-int
-__allocenv(int offset)
-{
- char **p;
- size_t required_len, new_len;
-
- if (offset == -1 || saveenv != environ) {
- char **ptr;
- for (ptr = environ, offset = 0; *ptr != NULL; ptr++)
- offset++;
- }
-
- /* one for potentially new entry one for NULL */
- required_len = offset + 2;
-
- if (required_len <= environ_malloced_len && saveenv == environ)
- return 0;
-
- /* Double the size of the arrays until we meet the requirement. */
- new_len = environ_malloced_len ? environ_malloced_len : 16;
- while (new_len < required_len)
- new_len <<= 1;
-
- if (saveenv == environ) { /* just increase size */
- if ((p = realloc(saveenv, new_len * sizeof(*p))) == NULL)
- return -1;
- (void)memset(&p[environ_malloced_len], 0,
- (new_len - environ_malloced_len) * sizeof(*p));
- saveenv = p;
- } else { /* get new space */
- free(saveenv);
- if ((saveenv = malloc(new_len * sizeof(*saveenv))) == NULL)
- return -1;
- (void)memcpy(saveenv, environ,
- (required_len - 2) * sizeof(*saveenv));
- (void)memset(&saveenv[required_len - 2], 0,
- (new_len - (required_len - 2)) * sizeof(*saveenv));
- }
- environ = saveenv;
-
- p = realloc(__environ_malloced, new_len * sizeof(*p));
- if (p == NULL)
+ l_name = __envvarnamelen(name, false);
+ if (l_name == 0)
return -1;
- (void)memset(&p[environ_malloced_len], 0,
- (new_len - environ_malloced_len) * sizeof(*p));
- environ_malloced_len = new_len;
- __environ_malloced = p;
-
- return 0;
-}
-
-/*
- * Handle the case where a program tried to cleanup the environment
- * by setting *environ = NULL; we attempt to cleanup all the malloced
- * environ entries and we make sure that the entry following the new
- * entry is NULL.
- */
-void
-__scrubenv(int offset)
-{
- if (environ[++offset] == NULL)
- return;
-
- for (; environ[offset]; offset++) {
- if (environ[offset] == __environ_malloced[offset])
- free(__environ_malloced[offset]);
- environ[offset] = __environ_malloced[offset] = NULL;
- }
-}
-
-/*
- * __findenv --
- * Returns pointer to value associated with name, if any, else NULL.
- * Sets offset to be the offset of the name/value combination in the
- * environmental array, for use by setenv(3) and unsetenv(3).
- * Explicitly removes '=' in argument name.
- *
- * This routine *should* be a static; don't use it.
- */
-char *
-__findenv(const char *name, int *offset)
-{
- size_t len;
- const char *np;
- char **p, *c;
-
- if (name == NULL || environ == NULL)
- return NULL;
- for (np = name; *np && *np != '='; ++np)
- continue;
- len = np - name;
- for (p = environ; (c = *p) != NULL; ++p)
- if (strncmp(c, name, len) == 0 && c[len] == '=') {
- *offset = (int)(p - environ);
- return c + len + 1;
+ rv = -1;
+ if (__readlockenv()) {
+ const char *value;
+
+ value = __findenv(name, l_name);
+ if (value != NULL) {
+ if (strlcpy(buf, value, len) < len) {
+ rv = 0;
+ } else {
+ errno = ERANGE;
+ }
+ } else {
+ errno = ENOENT;
}
- *offset = (int)(p - environ);
- return NULL;
+ (void)__unlockenv();
+ }
+
+ return rv;
}
Index: lib/libc/stdlib/local.h
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/local.h,v
retrieving revision 1.5
diff -u -r1.5 local.h
--- lib/libc/stdlib/local.h 3 Nov 2010 15:01:07 -0000 1.5
+++ lib/libc/stdlib/local.h 13 Nov 2010 21:32:44 -0000
@@ -24,13 +24,13 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-char *__findenv(const char *, int *);
-int __allocenv(int);
-void __scrubenv(int);
+/* Environment handling. */
-#ifdef _REENTRANT
-extern rwlock_t __environ_lock;
-#endif
+#include <sys/types.h>
+#include <stdbool.h>
-extern char **environ;
-extern char **__environ_malloced;
+extern size_t __envvarnamelen(const char *str, bool withequal);
+
+extern void __freeenvvar(char *envvar);
+extern char *__allocenvvar(size_t length);
+extern bool __canoverwriteenvvar(char *envvar, size_t length);
Index: lib/libc/stdlib/putenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/putenv.c,v
retrieving revision 1.18
diff -u -r1.18 putenv.c
--- lib/libc/stdlib/putenv.c 3 Nov 2010 15:01:07 -0000 1.18
+++ lib/libc/stdlib/putenv.c 13 Nov 2010 21:32:44 -0000
@@ -44,6 +44,8 @@
#include <errno.h>
#include <stdlib.h>
#include <string.h>
+
+#include "env.h"
#include "reentrant.h"
#include "local.h"
@@ -54,35 +56,32 @@
int
putenv(char *str)
{
- char *p;
- int offset;
+ size_t l_name;
+ int rv;
_DIAGASSERT(str != NULL);
- if (str == NULL || strchr(str, '=') == NULL || *str == '=') {
+ l_name = __envvarnamelen(str, true);
+ if (l_name == 0) {
errno = EINVAL;
return -1;
}
- if (rwlock_wrlock(&__environ_lock) != 0)
- return -1;
-
- p = __findenv(str, &offset);
+ rv = -1;
+ if (__writelockenv()) {
+ ssize_t offset;
+
+ offset = __getenvslot(str, l_name, true);
+ if (offset != -1) {
+ if (environ[offset] != NULL)
+ __freeenvvar(environ[offset]);
+ environ[offset] = str;
- if (__allocenv(offset) == -1)
- goto bad;
+ rv = 0;
+ }
- if (p != NULL && environ[offset] == __environ_malloced[offset]) {
- free(__environ_malloced[offset]);
- __environ_malloced[offset] = NULL;
+ (void)__unlockenv();
}
- environ[offset] = str;
- if (p == NULL)
- __scrubenv(offset);
- rwlock_unlock(&__environ_lock);
- return 0;
-bad:
- rwlock_unlock(&__environ_lock);
- return -1;
+ return rv;
}
Index: lib/libc/stdlib/setenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.42
diff -u -r1.42 setenv.c
--- lib/libc/stdlib/setenv.c 3 Nov 2010 15:01:07 -0000 1.42
+++ lib/libc/stdlib/setenv.c 13 Nov 2010 21:32:44 -0000
@@ -44,6 +44,8 @@
#include <errno.h>
#include <stdlib.h>
#include <string.h>
+
+#include "env.h"
#include "reentrant.h"
#include "local.h"
@@ -59,36 +61,33 @@
int
setenv(const char *name, const char *value, int rewrite)
{
- char *c, *f;
- size_t l_value, size;
- int offset;
+ size_t l_name, l_value, length;
+ ssize_t offset;
+ char *envvar;
_DIAGASSERT(name != NULL);
_DIAGASSERT(value != NULL);
- if (name == NULL || value == NULL) {
- errno = EINVAL;
- return -1;
- }
-
- size = strcspn(name, "=");
- if (size == 0 || name[size] != '\0') {
+ l_name = __envvarnamelen(name, false);
+ if (l_name == 0 || value == NULL) {
errno = EINVAL;
return -1;
}
- if (rwlock_wrlock(&__environ_lock) != 0)
+ if (!__writelockenv())
return -1;
- /* find if already exists */
- f = __findenv(name, &offset);
-
- if (__allocenv(offset) == -1)
+ /* Find slot in the enviroment. */
+ offset = __getenvslot(name, l_name, true);
+ if (offset == -1)
goto bad;
l_value = strlen(value);
+ length = l_name + l_value + 2;
- if (f != NULL) {
+ /* Handle overwriting a current environt variable. */
+ envvar = environ[offset];
+ if (envvar != NULL) {
if (!rewrite)
goto good;
/*
@@ -96,35 +95,34 @@
* whether there is enough space. If so simply overwrite the
* existing value.
*/
- if (environ[offset] == __environ_malloced[offset] &&
- strlen(f) >= l_value) {
- c = f;
+ if (__canoverwriteenvvar(envvar, length)) {
+ envvar += l_name + 1;
goto copy;
}
}
- /* name + `=' + value */
- if ((c = malloc(size + l_value + 2)) == NULL)
+
+ /* Allocate memory for name + `=' + value. */
+ if ((envvar = __allocenvvar(length)) == NULL)
goto bad;
- if (environ[offset] == __environ_malloced[offset])
- free(__environ_malloced[offset]);
+ if (environ[offset] != NULL)
+ __freeenvvar(environ[offset]);
- environ[offset] = c;
- __environ_malloced[offset] = c;
+ environ[offset] = envvar;
- if (f == NULL)
- __scrubenv(offset);
+ (void)memcpy(envvar, name, l_name);
- (void)memcpy(c, name, size);
- c += size;
- *c++ = '=';
+ envvar += l_name;
+ *envvar++ = '=';
copy:
- (void)memcpy(c, value, l_value + 1);
+ (void)memcpy(envvar, value, l_value + 1);
+
good:
- rwlock_unlock(&__environ_lock);
+ (void)__unlockenv();
return 0;
+
bad:
- rwlock_unlock(&__environ_lock);
+ (void)__unlockenv();
return -1;
}
Index: lib/libc/stdlib/system.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/system.c,v
retrieving revision 1.22
diff -u -r1.22 system.c
--- lib/libc/stdlib/system.c 27 Aug 2008 06:45:02 -0000 1.22
+++ lib/libc/stdlib/system.c 13 Nov 2010 21:32:44 -0000
@@ -46,12 +46,9 @@
#include <stdlib.h>
#include <unistd.h>
#include <paths.h>
-#include "reentrant.h"
-#ifdef _REENTRANT
-extern rwlock_t __environ_lock;
-#endif
-extern char **environ;
+#include "env.h"
+#include "reentrant.h"
int
system(command)
@@ -93,10 +90,10 @@
return -1;
}
- rwlock_rdlock(&__environ_lock);
+ (void)__readlockenv();
switch(pid = vfork()) {
case -1: /* error */
- rwlock_unlock(&__environ_lock);
+ (void)__unlockenv();
sigaction(SIGINT, &intsa, NULL);
sigaction(SIGQUIT, &quitsa, NULL);
(void)sigprocmask(SIG_SETMASK, &omask, NULL);
@@ -108,7 +105,7 @@
execve(_PATH_BSHELL, __UNCONST(argp), environ);
_exit(127);
}
- rwlock_unlock(&__environ_lock);
+ (void)__unlockenv();
while (waitpid(pid, &pstat, 0) == -1) {
if (errno != EINTR) {
Index: lib/libc/stdlib/unsetenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/unsetenv.c,v
retrieving revision 1.9
diff -u -r1.9 unsetenv.c
--- lib/libc/stdlib/unsetenv.c 30 Sep 2010 12:41:33 -0000 1.9
+++ lib/libc/stdlib/unsetenv.c 13 Nov 2010 21:32:44 -0000
@@ -45,6 +45,8 @@
#include <stdlib.h>
#include <string.h>
#include <bitstring.h>
+
+#include "env.h"
#include "reentrant.h"
#include "local.h"
@@ -55,36 +57,50 @@
int
unsetenv(const char *name)
{
- int offset;
+ size_t l_name;
+ ssize_t r_offset, w_offset;
_DIAGASSERT(name != NULL);
- if (name == NULL || *name == '\0' || strchr(name, '=') != NULL) {
+ l_name = __envvarnamelen(name, false);
+ if (l_name == 0) {
errno = EINVAL;
return -1;
}
- if (rwlock_wrlock(&__environ_lock) != 0)
+ if (!__writelockenv())
return -1;
- if (__allocenv(-1) == -1) {
- rwlock_unlock(&__environ_lock);
- return -1;
+ /* Search for the given name in the environment. */
+ r_offset = __getenvslot(name, l_name, false);
+ if (r_offset == -1) {
+ /* Not found. */
+ (void)__unlockenv();
+ return 0;
}
+ __freeenvvar(environ[r_offset]);
- while (__findenv(name, &offset) != NULL) { /* if set multiple times */
- if (environ[offset] == __environ_malloced[offset])
- free(__environ_malloced[offset]);
-
- while (environ[offset] != NULL) {
- environ[offset] = environ[offset + 1];
- __environ_malloced[offset] =
- __environ_malloced[offset + 1];
- offset++;
+ /*
+ * Remove all matches from the environment and free the associated
+ * memory if possible.
+ */
+ w_offset = r_offset;
+ while (environ[++r_offset] != NULL) {
+ if (strncmp(environ[r_offset], name, l_name) != 0 ||
+ environ[r_offset][l_name] != '=') {
+ /* Not a match, keep this entry. */
+ environ[w_offset++] = environ[r_offset];
+ } else {
+ /* We found another match. */
+ __freeenvvar(environ[r_offset]);
}
- __environ_malloced[offset] = NULL;
}
- rwlock_unlock(&__environ_lock);
+ /* Clear out remaining stale entries in the environment vector. */
+ do {
+ environ[w_offset++] = NULL;
+ } while (w_offset < r_offset);
+
+ (void)__unlockenv();
return 0;
}
Index: tests/lib/libc/stdlib/t_environment.c
===================================================================
RCS file: /cvsroot/src/tests/lib/libc/stdlib/t_environment.c,v
retrieving revision 1.9
diff -u -r1.9 t_environment.c
--- tests/lib/libc/stdlib/t_environment.c 13 Nov 2010 21:08:36 -0000
1.9
+++ tests/lib/libc/stdlib/t_environment.c 13 Nov 2010 21:32:48 -0000
@@ -174,11 +174,7 @@
{
ATF_CHECK(setenv("EVIL", "very=bad", 1) != -1);
ATF_CHECK_STREQ(getenv("EVIL"), "very=bad");
-
- atf_tc_expect_fail("getenv(3) doesn't check variable names properly");
ATF_CHECK(getenv("EVIL=very") == NULL);
-
- atf_tc_expect_pass();
ATF_CHECK(unsetenv("EVIL") != -1);
}
Attachment:
pgp5CwExzY3lb.pgp
Description: PGP signature