environment handling fixes

* AC_CHECK_FUNCS: +setenv +unsetenv -putenv
  (the check for putenv() wasn't used anyway)
* prefer setenv() to putenv() in cmd_setenv()
* prefer unsetenv() to putenv() in cmd_unsetenv()
  - putenv("FOO") isn't legit everywhere
  - putenv("FOO=") will only work on MinGW
* make the getenv command return an empty output if the variable wasn't found
* while here, split and sort AC_CHECK_FUNCS

|cos| on #ratpoison reported that environment variables weren't
properly removed, and proposed a different fix.  thanks!

2 files changed, 42 insertions, 24 deletions

diff --git a/configure.in b/configure.in
index 08f4ee8..b60e103 100644
--- a/configure.in
+++ b/configure.in
@@ -146,7 +146,8 @@ AC_CHECK_HEADERS(unistd.h stdarg.h)
 dnl Checks for typedefs, structures, and compiler characteristics.
 
 dnl Checks for library functions.
-AC_CHECK_FUNCS(getopt getopt_long setsid setpgid setpgrp putenv vsnprintf usleep getline)
+AC_CHECK_FUNCS(getline getopt getopt_long setenv setpgid setpgrp setsid)
+AC_CHECK_FUNCS(unsetenv usleep vsnprintf)
 
 AC_TYPE_SIGNAL
 
diff --git a/src/actions.c b/src/actions.c
index bf381c8..cc736dd 100644
--- a/src/actions.c
+++ b/src/actions.c
@@ -4125,28 +4125,36 @@ set_bwcolor (struct cmdarg **args)
 cmdret *
 cmd_setenv (int interactive UNUSED, struct cmdarg **args)
 {
+
+  const char *var = ARG_STRING(0), *val = ARG_STRING(1);
+  int ret;
+
+#ifdef HAVE_SETENV
+
+  PRINT_DEBUG(("setenv (\"%s\", \"%s\", 1)\n", var, val));
+  ret = setenv (var, val, 1);
+
+#else /* not HAVE_SETENV */
+
   struct sbuf *env;
 
   /* Setup the environment string. */
-  env = sbuf_new(0);
+  env = sbuf_new (strlen (var) + 1 + strlen (val) + 1);
 
-  sbuf_concat (env, ARG_STRING(0));
+  sbuf_copy (env, var);
   sbuf_concat (env, "=");
-  sbuf_concat (env, ARG_STRING(1));
+  sbuf_concat (env, val);
 
-  /* Stick it in the environment. */
-  PRINT_DEBUG(("%s\n", sbuf_get(env)));
-  putenv (sbuf_get (env));
+  /* Stick it in the environment.  We must *not* free it. */
+  PRINT_DEBUG(("putenv(\"%s\")\n", sbuf_get(env)));
+  ret = putenv (sbuf_free_struct (env));
 
-  /* According to the docs, the actual string is placed in the
-     environment, not the data the string points to. This means
-     modifying the string (or freeing it) directly changes the
-     environment. So, don't free the environment string, just the sbuf
-     data structure. */
-  env->data = NULL;
-  sbuf_free (env);
+#endif /* not HAVE_SETENV */
 
-  return cmdret_new (RET_SUCCESS, NULL);
+  if (ret == -1)
+    return cmdret_new (RET_FAILURE, "cmd_setenv failed: %s", strerror(errno));
+  else
+    return cmdret_new (RET_SUCCESS, NULL);
 }
 
 cmdret *
@@ -4158,7 +4166,7 @@ cmd_getenv (int interactive UNUSED, struct cmdarg **args)
   if (value)
     return cmdret_new (RET_SUCCESS, "%s", value);
   else
-    return cmdret_new (RET_SUCCESS, "");
+    return cmdret_new (RET_FAILURE, NULL);
 }
 
 /* Thanks to Gergely Nagy <algernon@debian.org> for the original
@@ -4190,16 +4198,25 @@ cmd_chdir (int interactive UNUSED, struct cmdarg **args)
 cmdret *
 cmd_unsetenv (int interactive UNUSED, struct cmdarg **args)
 {
-  struct sbuf *s;
+  const char *var = ARG_STRING(0);
+  int ret;
 
-  /* Remove all instances of the env. var. We must add an '=' for it
-     to work on OpenBSD. */
-  s = sbuf_new(0);
-  sbuf_copy (s, ARG_STRING(0));
+  /* Use unsetenv() where possible since putenv("FOO") is not legit everywhere */
+#ifdef HAVE_UNSETENV
+  ret = unsetenv(var);
+#else
+  /* MinGW doesn't have unsetenv() and uses putenv("FOO=") */
+  struct sbuf *s = sbuf_new (strlen (var) + 1 + 1);
+  sbuf_copy (s, var);
   sbuf_concat (s, "=");
-  putenv (sbuf_get(s));
-  sbuf_free (s);
-  return cmdret_new (RET_SUCCESS, NULL);
+  /* let putenv() decide whether to call free() */
+  ret = putenv (sbuf_free_struct (s));
+#endif
+
+  if (ret == -1)
+    return cmdret_new (RET_FAILURE, "cmd_unsetenv failed: %s", strerror (errno));
+  else
+    return cmdret_new (RET_SUCCESS, NULL);
 }
 
 /* Thanks to Gergely Nagy <algernon@debian.org> for the original