Source-Changes-HG archive

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

[src/trunk]: src/lib/libc Improve and simplify implementation of *env(3) func...



details:   https://anonhg.NetBSD.org/src/rev/8a6580d062e8
branches:  trunk
changeset: 758741:8a6580d062e8
user:      tron <tron%NetBSD.org@localhost>
date:      Sun Nov 14 18:11:42 2010 +0000

description:
Improve and simplify implementation of *env(3) functions:
- Use RB tree to keep track of memory allocated via setenv(3) as
  suggested by Enami Tsugutomo in private e-mail.
  This simplifies the code a lot as we no longer need to keep the size
  of "environ" in sync with an array of allocated environment variables.
  It also makes it possible to free environment variables in unsetenv(3)
  if something has changed the order of the "environ" array.
- Fix a bug in getenv(3) and getenv_r(3) which would return bogus
  results e.g. for " getenv("A=B") " if an environment variable "A"
  with value "B=C" exists.
- Clean up the internal functions:
  - Don't expose the read/write lock for the environment to other parts
    of "libc". Provide locking functions instead.
  - Use "bool" to report success or failure.
  - Use "ssize_t" or "size_t" instead of "int" for indexes.
  - Provide internal functions with simpler interfaces e.g. don't
    combine return values and reference arguments.
  - Don't copy "environ" into an allocated block unless we really need
    to grow it.

Code reviewed by Joerg Sonnenberger and Christos Zoulas, tested by
Joerg Sonnenberger and me. These changes also fix problems in
zsh 4.3.* and pam_ssh according to Joerg.

diffstat:

 lib/libc/compat/stdlib/compat_unsetenv.c |   32 +-
 lib/libc/gen/popen.c                     |   26 +-
 lib/libc/include/env.h                   |   65 ++++++
 lib/libc/misc/initfini.c                 |    8 +-
 lib/libc/stdlib/Makefile.inc             |    4 +-
 lib/libc/stdlib/_env.c                   |  330 +++++++++++++++++++++++++++++++
 lib/libc/stdlib/getenv.c                 |  166 +++------------
 lib/libc/stdlib/local.h                  |   18 +-
 lib/libc/stdlib/putenv.c                 |   43 +--
 lib/libc/stdlib/setenv.c                 |   68 +++---
 lib/libc/stdlib/system.c                 |   17 +-
 lib/libc/stdlib/unsetenv.c               |   58 +++-
 12 files changed, 576 insertions(+), 259 deletions(-)

diffs (truncated from 1214 to 300 lines):

diff -r d0cf5e55243f -r 8a6580d062e8 lib/libc/compat/stdlib/compat_unsetenv.c
--- a/lib/libc/compat/stdlib/compat_unsetenv.c  Sun Nov 14 15:47:20 2010 +0000
+++ b/lib/libc/compat/stdlib/compat_unsetenv.c  Sun Nov 14 18:11:42 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: compat_unsetenv.c,v 1.2 2010/09/23 17:30:49 christos Exp $     */
+/*     $NetBSD: compat_unsetenv.c,v 1.3 2010/11/14 18:11:42 tron Exp $ */
 
 /*
  * Copyright (c) 1987, 1993
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = "from: @(#)setenv.c     8.1 (Berkeley) 6/4/93";
 #else
-__RCSID("$NetBSD: compat_unsetenv.c,v 1.2 2010/09/23 17:30:49 christos Exp $");
+__RCSID("$NetBSD: compat_unsetenv.c,v 1.3 2010/11/14 18:11:42 tron Exp $");
 #endif
 #endif /* LIBC_SCCS and not lint */
 
@@ -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();
+       }
 }
diff -r d0cf5e55243f -r 8a6580d062e8 lib/libc/gen/popen.c
--- a/lib/libc/gen/popen.c      Sun Nov 14 15:47:20 2010 +0000
+++ b/lib/libc/gen/popen.c      Sun Nov 14 18:11:42 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: popen.c,v 1.29 2006/10/15 16:12:02 christos Exp $      */
+/*     $NetBSD: popen.c,v 1.30 2010/11/14 18:11:42 tron Exp $  */
 
 /*
  * Copyright (c) 1988, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)popen.c    8.3 (Berkeley) 5/3/95";
 #else
-__RCSID("$NetBSD: popen.c,v 1.29 2006/10/15 16:12:02 christos Exp $");
+__RCSID("$NetBSD: popen.c,v 1.30 2010/11/14 18:11:42 tron Exp $");
 #endif
 #endif /* LIBC_SCCS and not lint */
 
@@ -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);
diff -r d0cf5e55243f -r 8a6580d062e8 lib/libc/include/env.h
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/lib/libc/include/env.h    Sun Nov 14 18:11:42 2010 +0000
@@ -0,0 +1,65 @@
+/*     $NetBSD: env.h,v 1.1 2010/11/14 18:11:42 tron Exp $     */
+
+/*-
+ * 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;
diff -r d0cf5e55243f -r 8a6580d062e8 lib/libc/misc/initfini.c
--- a/lib/libc/misc/initfini.c  Sun Nov 14 15:47:20 2010 +0000
+++ b/lib/libc/misc/initfini.c  Sun Nov 14 18:11:42 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: initfini.c,v 1.6 2010/06/28 21:58:02 joerg Exp $        */
+/*     $NetBSD: initfini.c,v 1.7 2010/11/14 18:11:42 tron Exp $         */
 
 /*-
  * Copyright (c) 2007 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: initfini.c,v 1.6 2010/06/28 21:58:02 joerg Exp $");
+__RCSID("$NetBSD: initfini.c,v 1.7 2010/11/14 18:11:42 tron Exp $");
 
 #ifdef _LIBC
 #include "namespace.h"
@@ -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();
 }
diff -r d0cf5e55243f -r 8a6580d062e8 lib/libc/stdlib/Makefile.inc
--- a/lib/libc/stdlib/Makefile.inc      Sun Nov 14 15:47:20 2010 +0000
+++ b/lib/libc/stdlib/Makefile.inc      Sun Nov 14 18:11:42 2010 +0000
@@ -1,10 +1,10 @@
-#      $NetBSD: Makefile.inc,v 1.74 2010/05/03 05:01:53 jruoho Exp $
+#      $NetBSD: Makefile.inc,v 1.75 2010/11/14 18:11:43 tron Exp $
 #      from: @(#)Makefile.inc  8.3 (Berkeley) 2/4/95
 
 # 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 \
diff -r d0cf5e55243f -r 8a6580d062e8 lib/libc/stdlib/_env.c
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/lib/libc/stdlib/_env.c    Sun Nov 14 18:11:42 2010 +0000
@@ -0,0 +1,330 @@
+/*     $NetBSD: _env.c,v 1.1 2010/11/14 18:11:43 tron Exp $ */
+
+/*-
+ * 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



Home | Main Index | Thread Index | Old Index