Subject: putenv(3) POSIX - XPG compliance
To: None <tech-userlevel@netbsd.org>
From: Brian Ginsbach <ginsbach@cray.com>
List: tech-userlevel
Date: 01/30/2003 08:47:12
I'm wondering what others out there think about this.  It appears
that the classic BSD implementation (since introduction in 4.3BSD-Reno)
is not POSIX or X/Open SUSv[1-3] (XPG[4-6]) compliant.  Currently
putenv(3) is a wrapper around setenv(3).  The problem is that the
standards (as I read them) give putenv(3) a different behavior that
is incompatible with setenv(3).

From the current standard at
(http://www.opengroup.org/onlinepubs/007904975/functions/putenv.html)

    DESCRIPTION
    
     The putenv() function shall use the string argument to set
     environment variable values. The string argument should point to a
     string of the form "name=value". The putenv() function shall make
     the value of the environment variable name equal to value by
     altering an existing variable or creating a new one. In either
     case, the string pointed to by string shall become part of the
     environment, so altering the string shall change the environment.
     The space used by string is no longer used once a new
     string-defining name is passed to putenv().

The critical part be "so altering the string shall change the
environment." The current implementation using setenv(3) creates
a new string or in other words makes a copy of the string argument.
This means that it is not possible to change the original string
and have the change be reflected in the environment.

The current NetBSD man page states "The putenv() function conforms
to X/Open Portability Guide Issue 4 (``XPG4'')".  Given the above
I'd say this is an inaccurate statement.  Note the XPG4 standard is
nearly identical to the above citation.

Now will it cause a lot of heartbreak to change the "classic" BSD
behavior of putenv(3) to match that of the standard?  If not I'd
like to propose the following change(s).  Comments?

Index: getenv.3
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/getenv.3,v
retrieving revision 1.13
diff -u -r1.13 getenv.3
--- getenv.3	2002/08/10 17:14:50	1.13
+++ getenv.3	2003/01/30 04:00:45
@@ -94,19 +94,28 @@
 If the variable does exist, the argument
 .Ar overwrite
 is tested; if
-.Ar overwrite is
-zero, the
+.Ar overwrite
+is zero, the
 variable is not reset, otherwise it is reset
 to the given
 .Ar value .
 .Pp
 The
 .Fn putenv
-function takes an argument of the form ``name=value'' and is
-equivalent to:
-.Bd -literal -offset indent
-setenv(name, value, 1);
-.Ed
+function takes an argument of the form ``name=value'' and
+makes the value of the environment variable
+.Ar name
+equal to
+.Ar value
+by altering an existing variable or creating a new one.
+.Ar string
+becomes part of the environment, so altering the string
+will change the environment.  The space used by
+.Ar string
+is no longer used once a new string defining
+.Ar name
+is passed to
+.Fn putenv .
 .Pp
 The
 .Fn unsetenv
@@ -136,6 +145,21 @@
 .Fn putenv
 failed because they were unable to allocate memory for the environment.
 .El
+.Sh NOTES
+A potential error is to call
+.Fn putenv
+with an automatic variable as the argument, then
+return from the calling function while
+.Ar string
+is still part of the environment.  For this reason,
+.Ar string
+should be declared static if it is declared within a function.
+.Pp
+The
+.Fn setenv
+function is preferred over the
+.Fn putenv
+function.
 .Sh SEE ALSO
 .Xr csh 1 ,
 .Xr sh 1 ,
Index: putenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/putenv.c,v
retrieving revision 1.11
diff -u -r1.11 putenv.c
--- putenv.c	2000/01/22 22:19:19	1.11
+++ putenv.c	2003/01/30 04:00:45
@@ -57,19 +57,16 @@
 putenv(str)
 	const char *str;
 {
-	char *p, *equal;
-	int rval;
+	int offset;
 
 	_DIAGASSERT(str != NULL);
 
-	if ((p = strdup(str)) == NULL)
-		return (-1);
-	if ((equal = strchr(p, '=')) == NULL) {
-		(void)free(p);
-		return (-1);
+	if (__findenv(str, &offset) == NULL) {
+		if ((offset = __newenv()) < 0) {
+			return (-1);
+		}
 	}
-	*equal = '\0';
-	rval = setenv(p, equal + 1, 1);
-	(void)free(p);
-	return (rval);
+
+	environ[offset] = str;
+	return (0);
 }
Index: setenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.21
diff -u -r1.21 setenv.c
--- setenv.c	2003/01/18 11:32:04	1.21
+++ setenv.c	2003/01/30 04:00:45
@@ -63,6 +63,43 @@
 extern char **environ;
 
 /*
+ * __newenv --
+ *	Returns the index of the new the environment array element.
+ *	The envrionment array is expanded by one either via malloc()
+ *	or realloc(), for use by putenv(3) and setenv(3).
+ *
+ *	This routine *should* be static; don't use it.
+ */
+
+int
+__newenv()
+{
+	static int alloced;			/* if allocated space before */
+	int cnt;
+	char **p;
+
+	for (p = environ, cnt = 0; *p; ++p, ++cnt);
+	if (alloced) {			/* just increase size */
+		p = realloc(environ, (size_t)(sizeof(char *) * (cnt + 2)));
+		if (!p) {
+			return (-1);
+		}
+	}
+	else {				/* get new space */
+		alloced = 1;		/* copy old entries into it */
+		p = malloc((size_t)(sizeof(char *) * (cnt + 2)));
+		if (!p) {
+			return (-1);
+		}
+		memcpy(p, environ, cnt * sizeof(char *));
+	}
+	environ = p;
+	environ[cnt + 1] = NULL;
+
+	return (cnt);
+}
+
+/*
  * setenv --
  *	Set the value of the environmental variable "name" to be
  *	"value".  If rewrite is set, replace any current value.
@@ -73,7 +110,6 @@
 	const char *value;
 	int rewrite;
 {
-	static int alloced;			/* if allocated space before */
 	char *c;
 	const char *cc;
 	size_t l_value;
@@ -98,30 +134,10 @@
 			return (0);
 		}
 	} else {					/* create new slot */
-		int cnt;
-		char **p;
-
-		for (p = environ, cnt = 0; *p; ++p, ++cnt);
-		if (alloced) {			/* just increase size */
-			environ = realloc(environ,
-			    (size_t)(sizeof(char *) * (cnt + 2)));
-			if (!environ) {
-				rwlock_unlock(&__environ_lock);
-				return (-1);
-			}
-		}
-		else {				/* get new space */
-			alloced = 1;		/* copy old entries into it */
-			p = malloc((size_t)(sizeof(char *) * (cnt + 2)));
-			if (!p) {
-				rwlock_unlock(&__environ_lock);
-				return (-1);
-			}
-			memcpy(p, environ, cnt * sizeof(char *));
-			environ = p;
+		if ((offset = __newenv()) < 0) {
+			rwlock_unlock(&__environ_lock);
+			return (-1);
 		}
-		environ[cnt + 1] = NULL;
-		offset = cnt;
 	}
 	for (cc = name; *cc && *cc != '='; ++cc)/* no `=' in name */
 		continue;

-- 
Brian Ginsbach                          Cray Inc.