Use putenv+unsetenv instead of modifying environ directly.

* admin/merge-gnulib (GNULIB_MODULES): Add putenv, unsetenv.
* lib/putenv.c, lib/unsetenv.c, m4/putenv.m4, m4/setenv.m4:
New files, copied automatically from gnulib.
* lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate.
* src/alloc.c (xputenv): New function.
* src/dbusbind.c (Fdbus_init_bus):
* src/emacs.c (main):
* src/xterm.c (x_term_init):
Use xputenv instead of setenv or putenv, to detect memory exhaustion.
* src/editfns.c (initial_tz): Move static var decl up.
(tzvalbuf_in_environ): New static var.
(init_editfns):	Initialize these two static vars.
(Fencode_time): Don't assume arbitrary limit on EMACS_INT width.
Save old TZ value on stack, if it's small.
(Fencode_time, set_time_zone_rule): Don't modify 'environ' directly;
instead, use xputenv+unsetenv to set and restore TZ.
(environbuf): Remove static var.  All uses removed.
(Fset_time_zone_rule): Do not save TZ and environ;
no longer needed here.
(set_time_zone_rule_tz1, set_time_zone_rule_tz2) [LOCALTIME_CACHE]:
Move to inside set_time_zone_rule; they don't need file scope any more.
(set_time_zone_rule): Maintain the TZ=value string separately.
(syms_of_editfns): Don't initialize initial_tz;
init_editfns now does it.
* src/emacs.c (dump_tz) [HAVE_TZSET]: Now const.
* src/lisp.h (xputenv): New decl.

Fixes: debbugs:13070

7 files changed, 138 insertions, 118 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 54985d33b59..0be3eee6726 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,29 @@
+2012-12-08  Paul Eggert  <eggert@cs.ucla.edu>
+
+        Use putenv+unsetenv instead of modifying environ directly (Bug#13070).
+        * alloc.c (xputenv): New function.
+        * dbusbind.c (Fdbus_init_bus):
+        * emacs.c (main):
+        * xterm.c (x_term_init):
+        Use xputenv instead of setenv or putenv, to detect memory exhaustion.
+        * editfns.c (initial_tz): Move static var decl up.
+        (tzvalbuf_in_environ): New static var.
+        (init_editfns): Initialize these two static vars.
+        (Fencode_time): Don't assume arbitrary limit on EMACS_INT width.
+        Save old TZ value on stack, if it's small.
+        (Fencode_time, set_time_zone_rule): Don't modify 'environ' directly;
+        instead, use xputenv+unsetenv to set and restore TZ.
+        (environbuf): Remove static var.  All uses removed.
+        (Fset_time_zone_rule): Do not save TZ and environ;
+        no longer needed here.
+        (set_time_zone_rule_tz1, set_time_zone_rule_tz2) [LOCALTIME_CACHE]:
+        Move to inside set_time_zone_rule; they don't need file scope any more.
+        (set_time_zone_rule): Maintain the TZ=value string separately.
+        (syms_of_editfns): Don't initialize initial_tz;
+        init_editfns now does it.
+        * emacs.c (dump_tz) [HAVE_TZSET]: Now const.
+        * lisp.h (xputenv): New decl.
+
 2012-12-08  Fabrice Popineau  <fabrice.popineau@gmail.com>
 
         * w32fns.c (emacs_abort): Don't do arithmetics on void pointers.
diff --git a/src/alloc.c b/src/alloc.c
index 0f105f87207..5a3ba465d81 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -820,6 +820,15 @@ xstrdup (const char *s)
   return p;
 }
 
+/* Like putenv, but (1) use the equivalent of xmalloc and (2) the
+   argument is a const pointer.  */
+
+void
+xputenv (char const *string)
+{
+  if (putenv ((char *) string) != 0)
+    memory_full (0);
+}
 
 /* Unwind for SAFE_ALLOCA */
 
diff --git a/src/dbusbind.c b/src/dbusbind.c
index 80086946fc4..da8bbb1e5d7 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -1203,7 +1203,7 @@ this connection to those buses.  */)
       xd_registered_buses = Fcons (Fcons (bus, val), xd_registered_buses);
 
       /* We do not want to abort.  */
-      putenv ((char *) "DBUS_FATAL_WARNINGS=0");
+      xputenv ("DBUS_FATAL_WARNINGS=0");
 
       /* Cleanup.  */
       dbus_error_free (&derror);
diff --git a/src/editfns.c b/src/editfns.c
index ffb9a38909b..7d179c8566a 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -78,6 +78,15 @@ Lisp_Object Qfield;
 
 static Lisp_Object Qboundary;
 
+/* The startup value of the TZ environment variable so it can be
+   restored if the user calls set-time-zone-rule with a nil
+   argument.  If null, the TZ environment variable was unset.  */
+static char const *initial_tz;
+
+/* True if the static variable tzvalbuf (defined in
+   set_time_zone_rule) is part of 'environ'.  */
+static bool tzvalbuf_in_environ;
+
 
 void
 init_editfns (void)
@@ -96,6 +105,9 @@ init_editfns (void)
     return;
 #endif /* not CANNOT_DUMP */
 
+  initial_tz = getenv ("TZ");
+  tzvalbuf_in_environ = 0;
+
   pw = getpwuid (getuid ());
 #ifdef MSDOS
   /* We let the real user name default to "root" because that's quite
@@ -1900,9 +1912,11 @@ usage: (encode-time SECOND MINUTE HOUR DAY MONTH YEAR &optional ZONE)  */)
     }
   else
     {
-      char tzbuf[100];
+      static char const tzbuf_format[] = "XXX%s%"pI"d:%02d:%02d";
+      char tzbuf[sizeof tzbuf_format + INT_STRLEN_BOUND (EMACS_INT)];
+      char *old_tzstring;
       const char *tzstring;
-      char **oldenv = environ, **newenv;
+      USE_SAFE_ALLOCA;
 
       if (EQ (zone, Qt))
         tzstring = "UTC0";
@@ -1914,13 +1928,20 @@ usage: (encode-time SECOND MINUTE HOUR DAY MONTH YEAR &optional ZONE)  */)
           EMACS_INT zone_hr = abszone / (60*60);
           int zone_min = (abszone/60) % 60;
           int zone_sec = abszone % 60;
-          sprintf (tzbuf, "XXX%s%"pI"d:%02d:%02d", "-" + (XINT (zone) < 0),
+          sprintf (tzbuf, tzbuf_format, "-" + (XINT (zone) < 0),
                    zone_hr, zone_min, zone_sec);
           tzstring = tzbuf;
         }
       else
         error ("Invalid time zone specification");
 
+      old_tzstring = getenv ("TZ");
+      if (old_tzstring)
+        {
+          char *buf = SAFE_ALLOCA (strlen (old_tzstring) + 1);
+          old_tzstring = strcpy (buf, old_tzstring);
+        }
+
       block_input ();
 
       /* Set TZ before calling mktime; merely adjusting mktime's returned
@@ -1929,15 +1950,12 @@ usage: (encode-time SECOND MINUTE HOUR DAY MONTH YEAR &optional ZONE)  */)
 
       value = mktime (&tm);
 
-      /* Restore TZ to previous value.  */
-      newenv = environ;
-      environ = oldenv;
+      set_time_zone_rule (old_tzstring);
 #ifdef LOCALTIME_CACHE
       tzset ();
 #endif
       unblock_input ();
-
-      xfree (newenv);
+      SAFE_FREE ();
     }
 
   if (value == (time_t) -1)
@@ -2067,16 +2085,6 @@ the data it can't find.  */)
   return list2 (zone_offset, zone_name);
 }
 
-/* This holds the value of `environ' produced by the previous
-   call to Fset_time_zone_rule, or 0 if Fset_time_zone_rule
-   has never been called.  */
-static char **environbuf;
-
-/* This holds the startup value of the TZ environment variable so it
-   can be restored if the user calls set-time-zone-rule with a nil
-   argument.  */
-static char *initial_tz;
-
 DEFUN ("set-time-zone-rule", Fset_time_zone_rule, Sset_time_zone_rule, 1, 1, 0,
        doc: /* Set the local time zone using TZ, a string specifying a time zone rule.
 If TZ is nil, use implementation-defined default time zone information.
@@ -2089,18 +2097,10 @@ only the former.  */)
   (Lisp_Object tz)
 {
   const char *tzstring;
-  char **old_environbuf;
 
   if (! (NILP (tz) || EQ (tz, Qt)))
     CHECK_STRING (tz);
 
-  block_input ();
-
-  /* When called for the first time, save the original TZ.  */
-  old_environbuf = environbuf;
-  if (!old_environbuf)
-    initial_tz = (char *) getenv ("TZ");
-
   if (NILP (tz))
     tzstring = initial_tz;
   else if (EQ (tz, Qt))
@@ -2108,106 +2108,97 @@ only the former.  */)
   else
     tzstring = SSDATA (tz);
 
+  block_input ();
   set_time_zone_rule (tzstring);
-  environbuf = environ;
-
   unblock_input ();
 
-  xfree (old_environbuf);
   return Qnil;
 }
 
-#ifdef LOCALTIME_CACHE
-
-/* These two values are known to load tz files in buggy implementations,
-   i.e. Solaris 1 executables running under either Solaris 1 or Solaris 2.
-   Their values shouldn't matter in non-buggy implementations.
-   We don't use string literals for these strings,
-   since if a string in the environment is in readonly
-   storage, it runs afoul of bugs in SVR4 and Solaris 2.3.
-   See Sun bugs 1113095 and 1114114, ``Timezone routines
-   improperly modify environment''.  */
-
-static char set_time_zone_rule_tz1[] = "TZ=GMT+0";
-static char set_time_zone_rule_tz2[] = "TZ=GMT+1";
-
-#endif
-
 /* Set the local time zone rule to TZSTRING.
-   This allocates memory into `environ', which it is the caller's
-   responsibility to free.  */
+
+   This function is not thread-safe, partly because putenv, unsetenv
+   and tzset are not, and partly because of the static storage it
+   updates.  Other threads that invoke localtime etc. may be adversely
+   affected while this function is executing.  */
 
 void
 set_time_zone_rule (const char *tzstring)
 {
-  ptrdiff_t envptrs;
-  char **from, **to, **newenv;
+  /* A buffer holding a string of the form "TZ=value", intended
+     to be part of the environment.  */
+  static char *tzvalbuf;
+  static ptrdiff_t tzvalbufsize;
 
-  /* Make the ENVIRON vector longer with room for TZSTRING.  */
-  for (from = environ; *from; from++)
-    continue;
-  envptrs = from - environ + 2;
-  newenv = to = xmalloc (envptrs * sizeof *newenv
-                         + (tzstring ? strlen (tzstring) + 4 : 0));
+  int tzeqlen = sizeof "TZ=" - 1;
+
+#ifdef LOCALTIME_CACHE
+  /* These two values are known to load tz files in buggy implementations,
+     i.e., Solaris 1 executables running under either Solaris 1 or Solaris 2.
+     Their values shouldn't matter in non-buggy implementations.
+     We don't use string literals for these strings,
+     since if a string in the environment is in readonly
+     storage, it runs afoul of bugs in SVR4 and Solaris 2.3.
+     See Sun bugs 1113095 and 1114114, ``Timezone routines
+     improperly modify environment''.  */
+
+  static char set_time_zone_rule_tz[][sizeof "TZ=GMT+0"]
+    = { "TZ=GMT+0", "TZ=GMT+1" };
+
+  /* In SunOS 4.1.3_U1 and 4.1.4, if TZ has a value like
+     "US/Pacific" that loads a tz file, then changes to a value like
+     "XXX0" that does not load a tz file, and then changes back to
+     its original value, the last change is (incorrectly) ignored.
+     Also, if TZ changes twice in succession to values that do
+     not load a tz file, tzset can dump core (see Sun bug#1225179).
+     The following code works around these bugs.  */
 
-  /* Add TZSTRING to the end of environ, as a value for TZ.  */
   if (tzstring)
     {
-      char *t = (char *) (to + envptrs);
-      strcpy (t, "TZ=");
-      strcat (t, tzstring);
-      *to++ = t;
+      /* Temporarily set TZ to a value that loads a tz file
+         and that differs from tzstring.  */
+      bool eq0 = strcmp (tzstring, set_time_zone_rule_tz[0] + tzeqlen) == 0;
+      xputenv (set_time_zone_rule_tz[eq0]);
     }
+  else
+    {
+      /* The implied tzstring is unknown, so temporarily set TZ to
+         two different values that each load a tz file.  */
+      xputenv (set_time_zone_rule_tz[0]);
+      tzset ();
+      xputenv (set_time_zone_rule_tz[1]);
+    }
+  tzset ();
+#endif
 
-  /* Copy the old environ vector elements into NEWENV,
-     but don't copy the TZ variable.
-     So we have only one definition of TZ, which came from TZSTRING.  */
-  for (from = environ; *from; from++)
-    if (strncmp (*from, "TZ=", 3) != 0)
-      *to++ = *from;
-  *to = 0;
-
-  environ = newenv;
+  if (!tzstring)
+    {
+      unsetenv ("TZ");
+      tzvalbuf_in_environ = 0;
+    }
+  else
+    {
+      ptrdiff_t tzstringlen = strlen (tzstring);
 
-  /* If we do have a TZSTRING, NEWENV points to the vector slot where
-     the TZ variable is stored.  If we do not have a TZSTRING,
-     TO points to the vector slot which has the terminating null.  */
+      if (tzvalbufsize <= tzeqlen + tzstringlen)
+        {
+          unsetenv ("TZ");
+          tzvalbuf_in_environ = 0;
+          tzvalbuf = xpalloc (tzvalbuf, &tzvalbufsize,
+                              tzeqlen + tzstringlen - tzvalbufsize + 1, -1, 1);
+          memcpy (tzvalbuf, "TZ=", tzeqlen);
+        }
 
-#ifdef LOCALTIME_CACHE
-  {
-    /* In SunOS 4.1.3_U1 and 4.1.4, if TZ has a value like
-       "US/Pacific" that loads a tz file, then changes to a value like
-       "XXX0" that does not load a tz file, and then changes back to
-       its original value, the last change is (incorrectly) ignored.
-       Also, if TZ changes twice in succession to values that do
-       not load a tz file, tzset can dump core (see Sun bug#1225179).
-       The following code works around these bugs.  */
-
-    if (tzstring)
-      {
-        /* Temporarily set TZ to a value that loads a tz file
-           and that differs from tzstring.  */
-        char *tz = *newenv;
-        *newenv = (strcmp (tzstring, set_time_zone_rule_tz1 + 3) == 0
-                   ? set_time_zone_rule_tz2 : set_time_zone_rule_tz1);
-        tzset ();
-        *newenv = tz;
-      }
-    else
-      {
-        /* The implied tzstring is unknown, so temporarily set TZ to
-           two different values that each load a tz file.  */
-        *to = set_time_zone_rule_tz1;
-        to[1] = 0;
-        tzset ();
-        *to = set_time_zone_rule_tz2;
-        tzset ();
-        *to = 0;
-      }
+      strcpy (tzvalbuf + tzeqlen, tzstring);
 
-    /* Now TZ has the desired value, and tzset can be invoked safely.  */
-  }
+      if (!tzvalbuf_in_environ)
+        {
+          xputenv (tzvalbuf);
+          tzvalbuf_in_environ = 1;
+        }
+    }
 
+#ifdef LOCALTIME_CACHE
   tzset ();
 #endif
 }
@@ -4800,9 +4791,6 @@ Transposing beyond buffer boundaries is an error.  */)
 void
 syms_of_editfns (void)
 {
-  environbuf = 0;
-  initial_tz = 0;
-
   DEFSYM (Qbuffer_access_fontify_functions, "buffer-access-fontify-functions");
 
   DEFVAR_LISP ("inhibit-field-text-motion", Vinhibit_field_text_motion,
diff --git a/src/emacs.c b/src/emacs.c
index b2b193e3a4f..fbaf0355000 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -535,7 +535,7 @@ DEFUN ("invocation-directory", Finvocation_directory, Sinvocation_directory,
 #ifdef HAVE_TZSET
 /* A valid but unlikely value for the TZ environment value.
    It is OK (though a bit slower) if the user actually chooses this value.  */
-static char dump_tz[] = "UtC0";
+static char const dump_tz[] = "UtC0";
 #endif
 
 #ifndef ORDINARY_LINK
@@ -717,7 +717,7 @@ main (int argc, char **argv)
 
 #ifdef G_SLICE_ALWAYS_MALLOC
   /* This is used by the Cygwin build.  */
-  setenv ("G_SLICE", "always-malloc", 1);
+  xputenv ("G_SLICE=always-malloc");
 #endif
 
 #ifdef GNU_LINUX
@@ -803,9 +803,8 @@ main (int argc, char **argv)
 #ifdef HAVE_PERSONALITY_LINUX32
   if (dumping && ! getenv ("EMACS_HEAP_EXEC"))
     {
-      static char heapexec[] = "EMACS_HEAP_EXEC=true";
       /* Set this so we only do this once.  */
-      putenv (heapexec);
+      xputenv ("EMACS_HEAP_EXEC=true");
 
       /* A flag to turn off address randomization which is introduced
          in linux kernel shipped with fedora core 4 */
@@ -1309,7 +1308,7 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
      don't pollute Vglobal_environment.  */
   /* Setting LANG here will defeat the startup locale processing...  */
 #ifdef AIX
-  putenv ("LANG=C");
+  xputenv ("LANG=C");
 #endif
 
   init_buffer ();       /* Init default directory of main buffer.  */
diff --git a/src/lisp.h b/src/lisp.h
index 172a32e92a9..91fc3dfa1c6 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3594,6 +3594,7 @@ extern void *xnrealloc (void *, ptrdiff_t, ptrdiff_t);
 extern void *xpalloc (void *, ptrdiff_t *, ptrdiff_t, ptrdiff_t, ptrdiff_t);
 
 extern char *xstrdup (const char *);
+extern void xputenv (const char *);
 
 extern char *egetenv (const char *);
 
diff --git a/src/xterm.c b/src/xterm.c
index 8acd1d7843e..e9e99574663 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -9908,10 +9908,7 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name)
 
         /* Emacs can only handle core input events, so make sure
            Gtk doesn't use Xinput or Xinput2 extensions.  */
-        {
-          static char fix_events[] = "GDK_CORE_DEVICE_EVENTS=1";
-          putenv (fix_events);
-        }
+        xputenv ("GDK_CORE_DEVICE_EVENTS=1");
 
         /* Work around GLib bug that outputs a faulty warning. See
            https://bugzilla.gnome.org/show_bug.cgi?id=563627.  */