Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Improved implementation of *env(3) functions in "libc"



        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: pgp1NssXPuuNf.pgp
Description: PGP signature



Home | Main Index | Thread Index | Old Index