Re: Some more setenv(3) update

[Nicolas Joly <njoly%pasteur.fr@localhost>]

On Mon, Oct 11, 2010 at 11:01:30PM +0000, Christos Zoulas wrote:
> In article <20101011221824.GA423910%medusa.sis.pasteur.fr@localhost>,
> Nicolas Joly  <njoly%pasteur.fr@localhost> wrote:
> >-=-=-=-=-=-
> >
> >Following the recent setenv(3) update, i made the attached patch which
> >aims the following :
> >
> >- make it fails with EINVAL for bad name values, just like our
> >  unsetenv(3); following the opengroup online function description.
> >  http://www.opengroup.org/onlinepubs/009695399/functions/setenv.html
> >  Likewise do reject NULL value.
> >
> >- Do not strip (anymore) a leading `=' in provided value, mostly to
> >  give consistent results with putenv("var==val"). In both case getenv
> >  now return `=val'.
> >
> >The patch do include the corresponding testcases, and an updated
> >version of the man page.
> >
> >Comments ?
> 
> You could move the loop:
> 
>        for (cc = name; *cc && *cc != '='; ++cc)        /* no `=' in name */
>                continue;
> 
> before the initial test, and then check for *cc instead of scanning the
> name twice, once in strchr and a second time in strlen.

Right, but we then need to handle case for NULL name.

Here's an updated patch; which include needed adjustement for env(1),
from setenv to putenv.

-- 
Nicolas Joly

Biological Software and Databanks.
Institut Pasteur, Paris.

Index: lib/libc/stdlib/getenv.3
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/getenv.3,v
retrieving revision 1.22
diff -u -p -r1.22 getenv.3
--- lib/libc/stdlib/getenv.3    1 Oct 2010 20:57:50 -0000       1.22
+++ lib/libc/stdlib/getenv.3    12 Oct 2010 09:58:25 -0000
@@ -62,16 +62,14 @@ These functions set, unset and fetch env
 host
 .Em environment list .
 For compatibility with differing environment conventions,
-the given arguments
+the
+.Fn getenv
+or
+.Fn getenv_r
+given argument
 .Ar name
-and
-.Ar value
-may be appended and prepended,
-respectively,
-with an equal sign
-.Dq Li \&= ,
-except for
-.Fn unsetenv .
+may be appended with an equal sign
+.Dq Li \&= .
 .Pp
 The
 .Fn getenv
@@ -160,11 +158,18 @@ is successful, the string returned shoul
 The
 .Fa name
 argument to
+.Fn setenv
+or
 .Fn unsetenv
 is a null pointer, points to an empty string, or points to a string
 containing an
 .Dq Li \&=
 character.
+The
+.Fa value
+argument to
+.Fn setenv
+is a null pointer.
 .It Bq Er ENOMEM
 The function
 .Fn setenv
Index: lib/libc/stdlib/setenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.40
diff -u -p -r1.40 setenv.c
--- lib/libc/stdlib/setenv.c    2 Oct 2010 16:56:03 -0000       1.40
+++ lib/libc/stdlib/setenv.c    12 Oct 2010 09:58:25 -0000
@@ -67,6 +67,14 @@ setenv(const char *name, const char *val
        _DIAGASSERT(name != NULL);
        _DIAGASSERT(value != NULL);
 
+       for (cc = name; cc && *cc && *cc != '='; ++cc)
+               continue;
+
+       if (name == NULL || value == NULL || *name == '\0' || *cc == '=') {
+               errno = EINVAL;
+               return -1;
+       }
+
        if (rwlock_wrlock(&__environ_lock) != 0)
                return -1;
 
@@ -76,8 +84,6 @@ setenv(const char *name, const char *val
        if (__allocenv(offset) == -1)
                goto bad;
 
-       if (*value == '=')                      /* no `=' in value */
-               ++value;
        l_value = strlen(value);
 
        if (c != NULL) {
@@ -93,8 +99,6 @@ setenv(const char *name, const char *val
                        goto copy;
                }
        }
-       for (cc = name; *cc && *cc != '='; ++cc)        /* no `=' in name */
-               continue;
        size = cc - name;
        /* name + `=' + value */
        if ((c = malloc(size + l_value + 2)) == NULL)
Index: tests/lib/libc/stdlib/t_environment.c
===================================================================
RCS file: /cvsroot/src/tests/lib/libc/stdlib/t_environment.c,v
retrieving revision 1.2
diff -u -p -r1.2 t_environment.c
--- tests/lib/libc/stdlib/t_environment.c       1 Oct 2010 20:12:20 -0000       
1.2
+++ tests/lib/libc/stdlib/t_environment.c       12 Oct 2010 09:58:25 -0000
@@ -35,6 +35,7 @@
 __RCSID("$NetBSD: t_environment.c,v 1.2 2010/10/01 20:12:20 christos Exp $");
 
 #include <atf-c.h>
+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -71,6 +72,14 @@ ATF_TC_BODY(t_setenv, tc)
                unsetenv(name);
                ATF_CHECK(getenv(name) == NULL);
        }
+
+       ATF_CHECK_ERRNO(EINVAL, setenv(NULL, "val", 1) == -1);
+       ATF_CHECK_ERRNO(EINVAL, setenv("", "val", 1) == -1);
+       ATF_CHECK_ERRNO(EINVAL, setenv("v=r", "val", 1) == -1);
+       ATF_CHECK_ERRNO(EINVAL, setenv("var", NULL, 1) == -1);
+
+       ATF_CHECK(setenv("var", "=val", 1) == 0);
+       ATF_CHECK_STREQ(getenv("var"), "=val");
 }
 
 ATF_TC_BODY(t_putenv, tc)
Index: usr.bin/env/env.c
===================================================================
RCS file: /cvsroot/src/usr.bin/env/env.c,v
retrieving revision 1.17
diff -u -p -r1.17 env.c
--- usr.bin/env/env.c   21 Jul 2008 14:19:22 -0000      1.17
+++ usr.bin/env/env.c   12 Oct 2010 09:58:25 -0000
@@ -54,7 +54,7 @@ extern char **environ;
 int
 main(int argc, char **argv)
 {
-       char **ep, *p;
+       char **ep;
        char *cleanenv[1];
        int ch;
 
@@ -72,8 +72,8 @@ main(int argc, char **argv)
                        usage();
                }
 
-       for (argv += optind; *argv && (p = strchr(*argv, '=')); ++argv)
-               (void)setenv(*argv, ++p, 1);
+       for (argv += optind; *argv && strchr(*argv, '=') != NULL; ++argv)
+               (void)putenv(*argv);
 
        if (*argv) {
                /* return 127 if the command to be run could not be found; 126