Setenv/putenv changes break existing code.

Setenv/putenv changes break existing code.

[H.J. Lu]

Hi,

The recent setenv/putenv change break existing code like:

	p = malloc (100);
	.....
	putenv (p);
	free (p);

sice p is used directly in putenv. However, from the Unix standard:

The putenv() function uses the string argument to set environment
variable values. The string argument should point to a string of the
form "name=value". The putenv() function makes 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 becomes part of the environment, so altering the string will
change the environment. The space used by string is no longer used
once a new string-defining name is passed to putenv(). 

It seems to me that it is ok to free 'p' since it is no longer used
when returning from putenv ().

This change breaks gdm. I don't know if anything else is broken.


-- 
H.J. Lu (hjl@gnu.org)

Re: Setenv/putenv changes break existing code.

[Ulrich Drepper]

hjl@lucon.org (H.J. Lu) writes:

> [...] In either case, the string pointed to
> by string becomes part of the environment, so altering the string will
> change the environment. The space used by string is no longer used
> once a new string-defining name is passed to putenv(). 
> 
> It seems to me that it is ok to free 'p' since it is no longer used
> when returning from putenv ().

You are quoting the standard and still don't see that your argument is
wrong?  The putenv() code as it is now is correct.

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

Re: Setenv/putenv changes break existing code.

[Cristian Gafton]

On 2 Aug 1999, Ulrich Drepper wrote:

> You are quoting the standard and still don't see that your argument is
> wrong?  The putenv() code as it is now is correct.

Yes, the change might be correct, but it breaks existing binary code, and
again we have symbol versioning and again we are not using it...

I think attached patch should help this...

1999-08-03  Cristian Gafton  <gafton@redhat.com>

	* sysdeps/generic/putenv.c (__new_putenv): rename from putenv and default for
	GLIBC_2.1.2
	(_old_putenv): new function
	* stdlib/Versions: add putenv to new GLIBC 2.1.2

--- glibc-2.1/stdlib/Versions.gafton	Tue Aug  3 18:59:48 1999
+++ glibc-2.1/stdlib/Versions	Tue Aug  3 19:00:30 1999
@@ -90,4 +90,8 @@
     # i*
     imaxabs; imaxdiv;
   }
+  GLIBC_2.1.2 {
+    # p*
+    putenv;
+  }
 }
--- glibc-2.1/sysdeps/generic/putenv.c.gafton	Mon Aug  2 14:55:35 1999
+++ glibc-2.1/sysdeps/generic/putenv.c	Tue Aug  3 18:59:02 1999
@@ -50,7 +50,7 @@
 
 /* Put STRING, which is of the form "NAME=VALUE", in the environment.  */
 int
-putenv (string)
+__new_putenv (string)
      const char *string;
 {
   const char *const name_end = strchr (string, '=');
@@ -70,3 +70,32 @@
   __unsetenv (string);
   return 0;
 }
+
+/* Put STRING, which is of the form "NAME=VALUE", in the environment.  */
+int
+_old_putenv (string)
+     const char *string;
+{
+  const char *const name_end = strchr (string, '=');
+
+  if (name_end != NULL) {
+      char *name = alloca (name_end - string + 1);
+      memcpy (name, string, name_end - string);
+      name[name_end - string] = '\0';
+      return __add_to_environ (name, name_end + 1, NULL, 1);
+    }
+
+  __unsetenv (string);
+  return 0;
+}
+
+#if defined PIC && DO_VERSIONING
+default_symbol_version (__new_putenv, putenv, GLIBC_2.1.2);
+symbol_version (__old_putenv, putenv, GLIBC_2.0);
+#else
+# ifdef weak_alias
+weak_alias (__new_putenv, putenv)
+# endif
+#endif
+    
+    

Cristian
--
----------------------------------------------------------------------
Cristian Gafton     --     gafton@redhat.com      --     Red Hat, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 UNIX is user friendly. It's just selective about who its friends are.

Re: Setenv/putenv changes break existing code.

[Ulrich Drepper]

Cristian Gafton <gafton@redhat.com> writes:

> Yes, the change might be correct, but it breaks existing binary code, and
> again we have symbol versioning and again we are not using it...

This has nothing to do with this.  There are program with need the
correct functionality.  These programs are correct and they cannot run
because of the bug.  This is just like any other bug fix.  If somebody
depends on the bug he wrote broken code.  It simply hasn't shown up
until now.  It is not correct to hide these kind of things.  These
people might recompile the application and expect it to work but it
won't.  The versioning handling is not to keep bugs alive.  If you
have some program programs either fix them or have a simple
preloadable module which the user selectively can preload for some
program applications.

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

Re: Setenv/putenv changes break existing code.

[Geoff Keating]

> Date: Mon, 2 Aug 1999 18:00:00 -0700 (PDT)
> From: hjl@lucon.org (H.J. Lu)

> The recent setenv/putenv change break existing code like:
> 
> 	p = malloc (100);
> 	.....
> 	putenv (p);
> 	free (p);
> 
> sice p is used directly in putenv. However, from the Unix standard:
> 
> The putenv() function uses the string argument to set environment
> variable values. The string argument should point to a string of the
> form "name=value". The putenv() function makes 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 becomes part of the environment, so altering the string will
> change the environment. The space used by string is no longer used
> once a new string-defining name is passed to putenv(). 
> 
> It seems to me that it is ok to free 'p' since it is no longer used
> when returning from putenv ().

Surely freeing 'p' counts as altering *p?  And altering it "will
change the environment."

The last sentence means that if you do

p = malloc(100);
strcpy (p, "NAME=value");
putenv (p);
putenv ("NAME=some_other_value");

_now_ you can free `p', because you have redefined `NAME'.

-- 
Geoffrey Keating <geoffk@cygnus.com>