Re: Let's start using setenv() - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Let's start using setenv()
Date
Msg-id 2152500.1609260303@sss.pgh.pa.us
Whole thread Raw
In response to Re: Let's start using setenv()  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Let's start using setenv()
List pgsql-hackers
Since the cfbot seems happy with v1, here's a v2 that runs around
and converts all putenv() uses to setenv().  In some places there's
no notational savings, but it seems to me that standardizing
on just one way to do it is beneficial.

I'm not sure if there would be any value in revising win32env.c to
treat setenv, rather than putenv, as the primary use-case (and
switching to some corresponding Windows call instead of _putenv).
Perhaps some Windows hacker would be interested in considering that.
For myself, I'm happy with this version as it stands.

            regards, tom lane

diff --git a/configure b/configure
index 11a4284e5b..07529825d1 100755
--- a/configure
+++ b/configure
@@ -15959,12 +15959,29 @@ case $host_os in
         # Windows uses a specialised env handler
         mingw*)

+$as_echo "#define HAVE_SETENV 1" >>confdefs.h
+
+
 $as_echo "#define HAVE_UNSETENV 1" >>confdefs.h

+                ac_cv_func_setenv=yes
                 ac_cv_func_unsetenv=yes
                 ;;
         *)
-                ac_fn_c_check_func "$LINENO" "unsetenv" "ac_cv_func_unsetenv"
+                ac_fn_c_check_func "$LINENO" "setenv" "ac_cv_func_setenv"
+if test "x$ac_cv_func_setenv" = xyes; then :
+  $as_echo "#define HAVE_SETENV 1" >>confdefs.h
+
+else
+  case " $LIBOBJS " in
+  *" setenv.$ac_objext "* ) ;;
+  *) LIBOBJS="$LIBOBJS setenv.$ac_objext"
+ ;;
+esac
+
+fi
+
+ac_fn_c_check_func "$LINENO" "unsetenv" "ac_cv_func_unsetenv"
 if test "x$ac_cv_func_unsetenv" = xyes; then :
   $as_echo "#define HAVE_UNSETENV 1" >>confdefs.h

diff --git a/configure.ac b/configure.ac
index fc523c6aeb..7f855783f4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1757,11 +1757,13 @@ fi
 case $host_os in
         # Windows uses a specialised env handler
         mingw*)
+                AC_DEFINE(HAVE_SETENV, 1, [Define to 1 because replacement version used.])
                 AC_DEFINE(HAVE_UNSETENV, 1, [Define to 1 because replacement version used.])
+                ac_cv_func_setenv=yes
                 ac_cv_func_unsetenv=yes
                 ;;
         *)
-                AC_REPLACE_FUNCS([unsetenv])
+                AC_REPLACE_FUNCS([setenv unsetenv])
                 ;;
 esac

diff --git a/contrib/dblink/input/paths.source b/contrib/dblink/input/paths.source
index aab3a3b2bf..881a65314f 100644
--- a/contrib/dblink/input/paths.source
+++ b/contrib/dblink/input/paths.source
@@ -1,8 +1,8 @@
 -- Initialization that requires path substitution.

-CREATE FUNCTION putenv(text)
+CREATE FUNCTION setenv(text, text)
    RETURNS void
-   AS '@libdir@/regress@DLSUFFIX@', 'regress_putenv'
+   AS '@libdir@/regress@DLSUFFIX@', 'regress_setenv'
    LANGUAGE C STRICT;

 CREATE FUNCTION wait_pid(int)
@@ -11,4 +11,4 @@ CREATE FUNCTION wait_pid(int)
    LANGUAGE C STRICT;

 CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
-    AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;
+    AS $$SELECT setenv('PGSERVICEFILE', '@abs_srcdir@/' || $1)$$;
diff --git a/contrib/dblink/output/paths.source b/contrib/dblink/output/paths.source
index e1097f0996..8ed95e1f78 100644
--- a/contrib/dblink/output/paths.source
+++ b/contrib/dblink/output/paths.source
@@ -1,11 +1,11 @@
 -- Initialization that requires path substitution.
-CREATE FUNCTION putenv(text)
+CREATE FUNCTION setenv(text, text)
    RETURNS void
-   AS '@libdir@/regress@DLSUFFIX@', 'regress_putenv'
+   AS '@libdir@/regress@DLSUFFIX@', 'regress_setenv'
    LANGUAGE C STRICT;
 CREATE FUNCTION wait_pid(int)
    RETURNS void
    AS '@libdir@/regress@DLSUFFIX@'
    LANGUAGE C STRICT;
 CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
-    AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;
+    AS $$SELECT setenv('PGSERVICEFILE', '@abs_srcdir@/' || $1)$$;
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 515ae95fe1..64d6e85555 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1059,24 +1059,16 @@ pg_GSS_recvauth(Port *port)
         /*
          * Set default Kerberos keytab file for the Krb5 mechanism.
          *
-         * setenv("KRB5_KTNAME", pg_krb_server_keyfile, 0); except setenv()
-         * not always available.
+         * We don't overwrite any existing environment variable ... is that
+         * really a sane choice?
          */
-        if (getenv("KRB5_KTNAME") == NULL)
+        if (setenv("KRB5_KTNAME", pg_krb_server_keyfile, 0) != 0)
         {
-            size_t        kt_len = strlen(pg_krb_server_keyfile) + 14;
-            char       *kt_path = malloc(kt_len);
-
-            if (!kt_path ||
-                snprintf(kt_path, kt_len, "KRB5_KTNAME=%s",
-                         pg_krb_server_keyfile) != kt_len - 2 ||
-                putenv(kt_path) != 0)
-            {
-                ereport(LOG,
-                        (errcode(ERRCODE_OUT_OF_MEMORY),
-                         errmsg("out of memory")));
-                return STATUS_ERROR;
-            }
+            /* We assume the error must be "out of memory" */
+            ereport(LOG,
+                    (errcode(ERRCODE_OUT_OF_MEMORY),
+                     errmsg("out of memory")));
+            return STATUS_ERROR;
         }
     }

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index c39d67645c..088c1444c3 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -105,20 +105,6 @@ char       *localized_full_months[12 + 1];
 static bool CurrentLocaleConvValid = false;
 static bool CurrentLCTimeValid = false;

-/* Environment variable storage area */
-
-#define LC_ENV_BUFSIZE (NAMEDATALEN + 20)
-
-static char lc_collate_envbuf[LC_ENV_BUFSIZE];
-static char lc_ctype_envbuf[LC_ENV_BUFSIZE];
-
-#ifdef LC_MESSAGES
-static char lc_messages_envbuf[LC_ENV_BUFSIZE];
-#endif
-static char lc_monetary_envbuf[LC_ENV_BUFSIZE];
-static char lc_numeric_envbuf[LC_ENV_BUFSIZE];
-static char lc_time_envbuf[LC_ENV_BUFSIZE];
-
 /* Cache for collation-related knowledge */

 typedef struct
@@ -163,7 +149,6 @@ pg_perm_setlocale(int category, const char *locale)
 {
     char       *result;
     const char *envvar;
-    char       *envbuf;

 #ifndef WIN32
     result = setlocale(category, locale);
@@ -199,7 +184,7 @@ pg_perm_setlocale(int category, const char *locale)
      */
     if (category == LC_CTYPE)
     {
-        static char save_lc_ctype[LC_ENV_BUFSIZE];
+        static char save_lc_ctype[NAMEDATALEN + 20];

         /* copy setlocale() return value before callee invokes it again */
         strlcpy(save_lc_ctype, result, sizeof(save_lc_ctype));
@@ -216,16 +201,13 @@ pg_perm_setlocale(int category, const char *locale)
     {
         case LC_COLLATE:
             envvar = "LC_COLLATE";
-            envbuf = lc_collate_envbuf;
             break;
         case LC_CTYPE:
             envvar = "LC_CTYPE";
-            envbuf = lc_ctype_envbuf;
             break;
 #ifdef LC_MESSAGES
         case LC_MESSAGES:
             envvar = "LC_MESSAGES";
-            envbuf = lc_messages_envbuf;
 #ifdef WIN32
             result = IsoLocaleName(locale);
             if (result == NULL)
@@ -236,26 +218,19 @@ pg_perm_setlocale(int category, const char *locale)
 #endif                            /* LC_MESSAGES */
         case LC_MONETARY:
             envvar = "LC_MONETARY";
-            envbuf = lc_monetary_envbuf;
             break;
         case LC_NUMERIC:
             envvar = "LC_NUMERIC";
-            envbuf = lc_numeric_envbuf;
             break;
         case LC_TIME:
             envvar = "LC_TIME";
-            envbuf = lc_time_envbuf;
             break;
         default:
             elog(FATAL, "unrecognized LC category: %d", category);
-            envvar = NULL;        /* keep compiler quiet */
-            envbuf = NULL;
-            return NULL;
+            return NULL;        /* keep compiler quiet */
     }

-    snprintf(envbuf, LC_ENV_BUFSIZE - 1, "%s=%s", envvar, result);
-
-    if (putenv(envbuf))
+    if (setenv(envvar, result, 1) != 0)
         return NULL;

     return result;
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index f994c4216b..523d55dc65 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2355,8 +2355,7 @@ check_need_password(const char *authmethodlocal, const char *authmethodhost)
 void
 setup_pgdata(void)
 {
-    char       *pgdata_get_env,
-               *pgdata_set_env;
+    char       *pgdata_get_env;

     if (!pg_data)
     {
@@ -2386,8 +2385,11 @@ setup_pgdata(void)
      * need quotes otherwise on Windows because paths there are most likely to
      * have embedded spaces.
      */
-    pgdata_set_env = psprintf("PGDATA=%s", pg_data);
-    putenv(pgdata_set_env);
+    if (setenv("PGDATA", pg_data, 1) != 0)
+    {
+        pg_log_error("setenv failed");
+        exit(1);
+    }
 }


diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index fc07f1aba6..fcdc0213e4 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -889,11 +889,10 @@ do_start(void)
      */
 #ifndef WIN32
     {
-        static char env_var[32];
+        char        env_var[32];

-        snprintf(env_var, sizeof(env_var), "PG_GRANDPARENT_PID=%d",
-                 (int) getppid());
-        putenv(env_var);
+        snprintf(env_var, sizeof(env_var), "%d", (int) getppid());
+        setenv("PG_GRANDPARENT_PID", env_var, 1);
     }
 #endif

@@ -2340,12 +2339,10 @@ main(int argc, char **argv)
                 case 'D':
                     {
                         char       *pgdata_D;
-                        char       *env_var;

                         pgdata_D = pg_strdup(optarg);
                         canonicalize_path(pgdata_D);
-                        env_var = psprintf("PGDATA=%s", pgdata_D);
-                        putenv(env_var);
+                        setenv("PGDATA", pgdata_D, 1);

                         /*
                          * We could pass PGDATA just in an environment
@@ -2353,6 +2350,7 @@ main(int argc, char **argv)
                          * 'ps' display
                          */
                         pgdata_opt = psprintf("-D \"%s\" ", pgdata_D);
+                        free(pgdata_D);
                         break;
                     }
                 case 'e':
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 39bcaa8fe1..7eb56e7a29 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -97,20 +97,20 @@ get_control_data(ClusterInfo *cluster, bool live_check)
     if (getenv("LC_MESSAGES"))
         lc_messages = pg_strdup(getenv("LC_MESSAGES"));

-    pg_putenv("LC_COLLATE", NULL);
-    pg_putenv("LC_CTYPE", NULL);
-    pg_putenv("LC_MONETARY", NULL);
-    pg_putenv("LC_NUMERIC", NULL);
-    pg_putenv("LC_TIME", NULL);
+    unsetenv("LC_COLLATE");
+    unsetenv("LC_CTYPE");
+    unsetenv("LC_MONETARY");
+    unsetenv("LC_NUMERIC");
+    unsetenv("LC_TIME");
 #ifndef WIN32
-    pg_putenv("LANG", NULL);
+    unsetenv("LANG");
 #else
     /* On Windows the default locale may not be English, so force it */
-    pg_putenv("LANG", "en");
+    setenv("LANG", "en", 1);
 #endif
-    pg_putenv("LANGUAGE", NULL);
-    pg_putenv("LC_ALL", NULL);
-    pg_putenv("LC_MESSAGES", "C");
+    unsetenv("LANGUAGE");
+    unsetenv("LC_ALL");
+    setenv("LC_MESSAGES", "C", 1);

     /*
      * Check for clean shutdown
@@ -490,17 +490,31 @@ get_control_data(ClusterInfo *cluster, bool live_check)
     pclose(output);

     /*
-     * Restore environment variables
+     * Restore environment variables.  Note all but LANG and LC_MESSAGES were
+     * unset above.
      */
-    pg_putenv("LC_COLLATE", lc_collate);
-    pg_putenv("LC_CTYPE", lc_ctype);
-    pg_putenv("LC_MONETARY", lc_monetary);
-    pg_putenv("LC_NUMERIC", lc_numeric);
-    pg_putenv("LC_TIME", lc_time);
-    pg_putenv("LANG", lang);
-    pg_putenv("LANGUAGE", language);
-    pg_putenv("LC_ALL", lc_all);
-    pg_putenv("LC_MESSAGES", lc_messages);
+    if (lc_collate)
+        setenv("LC_COLLATE", lc_collate, 1);
+    if (lc_ctype)
+        setenv("LC_CTYPE", lc_ctype, 1);
+    if (lc_monetary)
+        setenv("LC_MONETARY", lc_monetary, 1);
+    if (lc_numeric)
+        setenv("LC_NUMERIC", lc_numeric, 1);
+    if (lc_time)
+        setenv("LC_TIME", lc_time, 1);
+    if (lang)
+        setenv("LANG", lang, 1);
+    else
+        unsetenv("LANG");
+    if (language)
+        setenv("LANGUAGE", language, 1);
+    if (lc_all)
+        setenv("LC_ALL", lc_all, 1);
+    if (lc_messages)
+        setenv("LC_MESSAGES", lc_messages, 1);
+    else
+        unsetenv("LC_MESSAGES");

     pg_free(lc_collate);
     pg_free(lc_ctype);
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 548d648e8c..5b566c14ef 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -193,7 +193,7 @@ parseCommandLine(int argc, char *argv[])
                  * Push the user name into the environment so pre-9.1
                  * pg_ctl/libpq uses it.
                  */
-                pg_putenv("PGUSER", os_info.user);
+                setenv("PGUSER", os_info.user, 1);
                 break;

             case 'v':
@@ -245,11 +245,11 @@ parseCommandLine(int argc, char *argv[])
         char       *pgoptions = psprintf("%s %s", FIX_DEFAULT_READ_ONLY,
                                          getenv("PGOPTIONS"));

-        pg_putenv("PGOPTIONS", pgoptions);
+        setenv("PGOPTIONS", pgoptions, 1);
         pfree(pgoptions);
     }
     else
-        pg_putenv("PGOPTIONS", FIX_DEFAULT_READ_ONLY);
+        setenv("PGOPTIONS", FIX_DEFAULT_READ_ONLY, 1);

     /* Get values from env if not already set */
     check_required_directory(&old_cluster.bindir, "PGBINOLD", false,
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index ee70243c2e..1842556274 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -436,7 +436,6 @@ void        end_progress_output(void);
 void        prep_status(const char *fmt,...) pg_attribute_printf(1, 2);
 void        check_ok(void);
 unsigned int str2uint(const char *str);
-void        pg_putenv(const char *var, const char *val);


 /* version.c */
diff --git a/src/bin/pg_upgrade/util.c b/src/bin/pg_upgrade/util.c
index a16c794261..9c9ba29124 100644
--- a/src/bin/pg_upgrade/util.c
+++ b/src/bin/pg_upgrade/util.c
@@ -241,39 +241,3 @@ str2uint(const char *str)
 {
     return strtoul(str, NULL, 10);
 }
-
-
-/*
- *    pg_putenv()
- *
- *    This is like putenv(), but takes two arguments.
- *    It also does unsetenv() if val is NULL.
- */
-void
-pg_putenv(const char *var, const char *val)
-{
-    if (val)
-    {
-#ifndef WIN32
-        char       *envstr;
-
-        envstr = psprintf("%s=%s", var, val);
-        putenv(envstr);
-
-        /*
-         * Do not free envstr because it becomes part of the environment on
-         * some operating systems.  See port/unsetenv.c::unsetenv.
-         */
-#else
-        SetEnvironmentVariableA(var, val);
-#endif
-    }
-    else
-    {
-#ifndef WIN32
-        unsetenv(var);
-#else
-        SetEnvironmentVariableA(var, "");
-#endif
-    }
-}
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 38b588882d..c545341cdd 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2296,17 +2296,8 @@ exec_command_setenv(PsqlScanState scan_state, bool active_branch,
         else
         {
             /* Set variable to the value of the next argument */
-            char       *newval;
-
-            newval = psprintf("%s=%s", envvar, envval);
-            putenv(newval);
+            setenv(envvar, envval, 1);
             success = true;
-
-            /*
-             * Do not free newval here, it will screw up the environment if
-             * you do. See putenv man page for details. That means we leak a
-             * bit of memory here, but not enough to worry about.
-             */
         }
         free(envvar);
         free(envval);
diff --git a/src/common/exec.c b/src/common/exec.c
index 78bb486f99..773afd080c 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -435,9 +435,6 @@ set_pglocale_pgservice(const char *argv0, const char *app)
 {
     char        path[MAXPGPATH];
     char        my_exec_path[MAXPGPATH];
-    char        env_path[MAXPGPATH + sizeof("PGSYSCONFDIR=")];    /* longer than
-                                                                 * PGLOCALEDIR */
-    char       *dup_path;

     /* don't set LC_ALL in the backend */
     if (strcmp(app, PG_TEXTDOMAIN("postgres")) != 0)
@@ -462,28 +459,15 @@ set_pglocale_pgservice(const char *argv0, const char *app)
     get_locale_path(my_exec_path, path);
     bindtextdomain(app, path);
     textdomain(app);
-
-    if (getenv("PGLOCALEDIR") == NULL)
-    {
-        /* set for libpq to use */
-        snprintf(env_path, sizeof(env_path), "PGLOCALEDIR=%s", path);
-        canonicalize_path(env_path + 12);
-        dup_path = strdup(env_path);
-        if (dup_path)
-            putenv(dup_path);
-    }
+    /* set for libpq to use, but don't override existing setting */
+    setenv("PGLOCALEDIR", path, 0);
 #endif

     if (getenv("PGSYSCONFDIR") == NULL)
     {
         get_etc_path(my_exec_path, path);
-
         /* set for libpq to use */
-        snprintf(env_path, sizeof(env_path), "PGSYSCONFDIR=%s", path);
-        canonicalize_path(env_path + 13);
-        dup_path = strdup(env_path);
-        if (dup_path)
-            putenv(dup_path);
+        setenv("PGSYSCONFDIR", path, 0);
     }
 }

diff --git a/src/common/restricted_token.c b/src/common/restricted_token.c
index dcc88a75c5..9058c5ad25 100644
--- a/src/common/restricted_token.c
+++ b/src/common/restricted_token.c
@@ -171,7 +171,7 @@ get_restricted_token(void)

         cmdline = pg_strdup(GetCommandLine());

-        putenv("PG_RESTRICT_EXEC=1");
+        setenv("PG_RESTRICT_EXEC", "1", 1);

         if ((restrictedToken = CreateRestrictedProcess(cmdline, &pi)) == 0)
         {
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index de8f838e53..ddaa9e8e18 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -473,6 +473,9 @@
 /* Define to 1 if you have the <security/pam_appl.h> header file. */
 #undef HAVE_SECURITY_PAM_APPL_H

+/* Define to 1 if you have the `setenv' function. */
+#undef HAVE_SETENV
+
 /* Define to 1 if you have the `setproctitle' function. */
 #undef HAVE_SETPROCTITLE

diff --git a/src/include/port.h b/src/include/port.h
index 5dfb00b07c..c631185c8b 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -447,8 +447,12 @@ extern size_t strnlen(const char *str, size_t maxlen);
 extern long random(void);
 #endif

+#ifndef HAVE_SETENV
+extern int    setenv(const char *name, const char *value, int overwrite);
+#endif
+
 #ifndef HAVE_UNSETENV
-extern void unsetenv(const char *name);
+extern int    unsetenv(const char *name);
 #endif

 #ifndef HAVE_SRANDOM
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index 59c7f35e3d..2ffe056a0f 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -490,7 +490,12 @@ extern void _dosmaperr(unsigned long);

 /* in port/win32env.c */
 extern int    pgwin32_putenv(const char *);
-extern void pgwin32_unsetenv(const char *);
+extern int    pgwin32_setenv(const char *name, const char *value, int overwrite);
+extern int    pgwin32_unsetenv(const char *name);
+
+#define putenv(x) pgwin32_putenv(x)
+#define setenv(x,y,z) pgwin32_setenv(x,y,z)
+#define unsetenv(x) pgwin32_unsetenv(x)

 /* in port/win32security.c */
 extern int    pgwin32_is_service(void);
@@ -499,9 +504,6 @@ extern int    pgwin32_is_admin(void);
 /* Windows security token manipulation (in src/common/exec.c) */
 extern BOOL AddUserToTokenDacl(HANDLE hToken);

-#define putenv(x) pgwin32_putenv(x)
-#define unsetenv(x) pgwin32_unsetenv(x)
-
 /* Things that exist in MinGW headers, but need to be added to MSVC */
 #ifdef _MSC_VER

diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c
index 6e1d25b1f4..49b2d8141f 100644
--- a/src/interfaces/ecpg/test/pg_regress_ecpg.c
+++ b/src/interfaces/ecpg/test/pg_regress_ecpg.c
@@ -147,8 +147,9 @@ ecpg_start_test(const char *testname,
              outfile_stdout,
              outfile_stderr);

-    appnameenv = psprintf("PGAPPNAME=ecpg/%s", testname_dash.data);
-    putenv(appnameenv);
+    appnameenv = psprintf("ecpg/%s", testname_dash.data);
+    setenv("PGAPPNAME", appnameenv, 1);
+    free(appnameenv);

     pid = spawn_process(cmd);

@@ -160,7 +161,6 @@ ecpg_start_test(const char *testname,
     }

     unsetenv("PGAPPNAME");
-    free(appnameenv);

     free(testname_dash.data);

diff --git a/src/port/setenv.c b/src/port/setenv.c
new file mode 100644
index 0000000000..d8a5647fb0
--- /dev/null
+++ b/src/port/setenv.c
@@ -0,0 +1,48 @@
+/*-------------------------------------------------------------------------
+ *
+ * setenv.c
+ *      setenv() emulation for machines without it
+ *
+ * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *      src/port/setenv.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "c.h"
+
+
+int
+setenv(const char *name, const char *value, int overwrite)
+{
+    char       *envstr;
+
+    /* Error conditions, per POSIX */
+    if (name == NULL || name[0] == '\0' || strchr(name, '=') != NULL ||
+        value == NULL)
+    {
+        errno = EINVAL;
+        return -1;
+    }
+
+    /* No work if variable exists and we're not to replace it */
+    if (overwrite == 0 && getenv(name) != NULL)
+        return 0;
+
+    /*
+     * Add or replace the value using putenv().  This will leak memory if the
+     * same variable is repeatedly redefined, but there's little we can do
+     * about that when sitting atop putenv().
+     */
+    envstr = (char *) malloc(strlen(name) + strlen(value) + 2);
+    if (!envstr)                /* not much we can do if no memory */
+        return -1;
+
+    sprintf(envstr, "%s=%s", name, value);
+
+    return putenv(envstr);
+}
diff --git a/src/port/unsetenv.c b/src/port/unsetenv.c
index f2028c2f83..a5f19f8db3 100644
--- a/src/port/unsetenv.c
+++ b/src/port/unsetenv.c
@@ -16,13 +16,20 @@
 #include "c.h"


-void
+int
 unsetenv(const char *name)
 {
     char       *envstr;

+    /* Error conditions, per POSIX */
+    if (name == NULL || name[0] == '\0' || strchr(name, '=') != NULL)
+    {
+        errno = EINVAL;
+        return -1;
+    }
+
     if (getenv(name) == NULL)
-        return;                    /* no work */
+        return 0;                /* no work */

     /*
      * The technique embodied here works if libc follows the Single Unix Spec
@@ -40,11 +47,12 @@ unsetenv(const char *name)

     envstr = (char *) malloc(strlen(name) + 2);
     if (!envstr)                /* not much we can do if no memory */
-        return;
+        return -1;

     /* Override the existing setting by forcibly defining the var */
     sprintf(envstr, "%s=", name);
-    putenv(envstr);
+    if (putenv(envstr))
+        return -1;

     /* Now we can clobber the variable definition this way: */
     strcpy(envstr, "=");
@@ -53,5 +61,5 @@ unsetenv(const char *name)
      * This last putenv cleans up if we have multiple zero-length names as a
      * result of unsetting multiple things.
      */
-    putenv(envstr);
+    return putenv(envstr);
 }
diff --git a/src/port/win32env.c b/src/port/win32env.c
index 177488cc67..f5bed67297 100644
--- a/src/port/win32env.c
+++ b/src/port/win32env.c
@@ -1,8 +1,10 @@
 /*-------------------------------------------------------------------------
  *
  * win32env.c
- *      putenv() and unsetenv() for win32, which update both process environment
- *      and caches in (potentially multiple) C run-time library (CRT) versions.
+ *      putenv(), setenv(), and unsetenv() for win32.
+ *
+ * These functions update both the process environment and caches in
+ * (potentially multiple) C run-time library (CRT) versions.
  *
  * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -16,6 +18,11 @@

 #include "c.h"

+
+/*
+ * Note that unlike POSIX putenv(), this doesn't use the passed-in string
+ * as permanent storage.
+ */
 int
 pgwin32_putenv(const char *envval)
 {
@@ -64,7 +71,7 @@ pgwin32_putenv(const char *envval)
     }
     *cp = '\0';
     cp++;
-    if (strlen(cp))
+    if (*cp)
     {
         /*
          * Only call SetEnvironmentVariable() when we are adding a variable,
@@ -110,16 +117,47 @@ pgwin32_putenv(const char *envval)
     return _putenv(envval);
 }

-void
+int
+pgwin32_setenv(const char *name, const char *value, int overwrite)
+{
+    int            res;
+    char       *envstr;
+
+    /* Error conditions, per POSIX */
+    if (name == NULL || name[0] == '\0' || strchr(name, '=') != NULL ||
+        value == NULL)
+    {
+        errno = EINVAL;
+        return -1;
+    }
+
+    /* No work if variable exists and we're not to replace it */
+    if (overwrite == 0 && getenv(name) != NULL)
+        return 0;
+
+    envstr = (char *) malloc(strlen(name) + strlen(value) + 2);
+    if (!envstr)                /* not much we can do if no memory */
+        return -1;
+
+    sprintf(envstr, "%s=%s", name, value);
+
+    res = pgwin32_putenv(envstr);
+    free(envstr);
+    return res;
+}
+
+int
 pgwin32_unsetenv(const char *name)
 {
+    int            res;
     char       *envbuf;

     envbuf = (char *) malloc(strlen(name) + 2);
     if (!envbuf)
-        return;
+        return -1;

     sprintf(envbuf, "%s=", name);
-    pgwin32_putenv(envbuf);
+    res = pgwin32_putenv(envbuf);
     free(envbuf);
+    return res;
 }
diff --git a/src/test/isolation/isolation_main.c b/src/test/isolation/isolation_main.c
index 50916b00dc..a6a64e7ec5 100644
--- a/src/test/isolation/isolation_main.c
+++ b/src/test/isolation/isolation_main.c
@@ -98,8 +98,9 @@ isolation_start_test(const char *testname,
         exit(2);
     }

-    appnameenv = psprintf("PGAPPNAME=isolation/%s", testname);
-    putenv(appnameenv);
+    appnameenv = psprintf("isolation/%s", testname);
+    setenv("PGAPPNAME", appnameenv, 1);
+    free(appnameenv);

     pid = spawn_process(psql_cmd);

@@ -111,7 +112,6 @@ isolation_start_test(const char *testname,
     }

     unsetenv("PGAPPNAME");
-    free(appnameenv);

     return pid;
 }
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 23d7d0beb2..866bc8c470 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -724,18 +724,6 @@ get_expectfile(const char *testname, const char *file)
     return NULL;
 }

-/*
- * Handy subroutine for setting an environment variable "var" to "val"
- */
-static void
-doputenv(const char *var, const char *val)
-{
-    char       *s;
-
-    s = psprintf("%s=%s", var, val);
-    putenv(s);
-}
-
 /*
  * Prepare environment variables for running regression tests
  */
@@ -746,7 +734,7 @@ initialize_environment(void)
      * Set default application_name.  (The test_function may choose to
      * override this, but if it doesn't, we have something useful in place.)
      */
-    putenv("PGAPPNAME=pg_regress");
+    setenv("PGAPPNAME", "pg_regress", 1);

     if (nolocale)
     {
@@ -769,7 +757,7 @@ initialize_environment(void)
          * variables unset; see PostmasterMain().
          */
 #if defined(WIN32) || defined(__CYGWIN__) || defined(__darwin__)
-        putenv("LANG=C");
+        setenv("LANG", "C", 1);
 #endif
     }

@@ -781,21 +769,21 @@ initialize_environment(void)
      */
     unsetenv("LANGUAGE");
     unsetenv("LC_ALL");
-    putenv("LC_MESSAGES=C");
+    setenv("LC_MESSAGES", "C", 1);

     /*
      * Set encoding as requested
      */
     if (encoding)
-        doputenv("PGCLIENTENCODING", encoding);
+        setenv("PGCLIENTENCODING", encoding, 1);
     else
         unsetenv("PGCLIENTENCODING");

     /*
      * Set timezone and datestyle for datetime-related tests
      */
-    putenv("PGTZ=PST8PDT");
-    putenv("PGDATESTYLE=Postgres, MDY");
+    setenv("PGTZ", "PST8PDT", 1);
+    setenv("PGDATESTYLE", "Postgres, MDY", 1);

     /*
      * Likewise set intervalstyle to ensure consistent results.  This is a bit
@@ -809,9 +797,10 @@ initialize_environment(void)

         if (!old_pgoptions)
             old_pgoptions = "";
-        new_pgoptions = psprintf("PGOPTIONS=%s %s",
+        new_pgoptions = psprintf("%s %s",
                                  old_pgoptions, my_pgoptions);
-        putenv(new_pgoptions);
+        setenv("PGOPTIONS", new_pgoptions, 1);
+        free(new_pgoptions);
     }

     if (temp_instance)
@@ -832,17 +821,17 @@ initialize_environment(void)
         unsetenv("PGDATA");
 #ifdef HAVE_UNIX_SOCKETS
         if (hostname != NULL)
-            doputenv("PGHOST", hostname);
+            setenv("PGHOST", hostname, 1);
         else
         {
             sockdir = getenv("PG_REGRESS_SOCK_DIR");
             if (!sockdir)
                 sockdir = make_temp_sockdir();
-            doputenv("PGHOST", sockdir);
+            setenv("PGHOST", sockdir, 1);
         }
 #else
         Assert(hostname != NULL);
-        doputenv("PGHOST", hostname);
+        setenv("PGHOST", hostname, 1);
 #endif
         unsetenv("PGHOSTADDR");
         if (port != -1)
@@ -850,7 +839,7 @@ initialize_environment(void)
             char        s[16];

             sprintf(s, "%d", port);
-            doputenv("PGPORT", s);
+            setenv("PGPORT", s, 1);
         }
     }
     else
@@ -864,7 +853,7 @@ initialize_environment(void)
          */
         if (hostname != NULL)
         {
-            doputenv("PGHOST", hostname);
+            setenv("PGHOST", hostname, 1);
             unsetenv("PGHOSTADDR");
         }
         if (port != -1)
@@ -872,10 +861,10 @@ initialize_environment(void)
             char        s[16];

             sprintf(s, "%d", port);
-            doputenv("PGPORT", s);
+            setenv("PGPORT", s, 1);
         }
         if (user != NULL)
-            doputenv("PGUSER", user);
+            setenv("PGUSER", user, 1);

         /*
          * However, we *don't* honor PGDATABASE, since we certainly don't wish
@@ -2431,7 +2420,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
                 fprintf(stderr, _("port %d apparently in use, trying %d\n"), port, port + 1);
                 port++;
                 sprintf(s, "%d", port);
-                doputenv("PGPORT", s);
+                setenv("PGPORT", s, 1);
             }
             else
                 break;
diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c
index dd8ad24564..5e503efa4a 100644
--- a/src/test/regress/pg_regress_main.c
+++ b/src/test/regress/pg_regress_main.c
@@ -91,8 +91,9 @@ psql_start_test(const char *testname,
         exit(2);
     }

-    appnameenv = psprintf("PGAPPNAME=pg_regress/%s", testname);
-    putenv(appnameenv);
+    appnameenv = psprintf("pg_regress/%s", testname);
+    setenv("PGAPPNAME", appnameenv, 1);
+    free(appnameenv);

     pid = spawn_process(psql_cmd);

@@ -104,7 +105,6 @@ psql_start_test(const char *testname,
     }

     unsetenv("PGAPPNAME");
-    free(appnameenv);

     return pid;
 }
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 09bc42a8c0..b8b3af4e95 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -624,22 +624,18 @@ make_tuple_indirect(PG_FUNCTION_ARGS)
     PG_RETURN_POINTER(newtup->t_data);
 }

-PG_FUNCTION_INFO_V1(regress_putenv);
+PG_FUNCTION_INFO_V1(regress_setenv);

 Datum
-regress_putenv(PG_FUNCTION_ARGS)
+regress_setenv(PG_FUNCTION_ARGS)
 {
-    MemoryContext oldcontext;
-    char       *envbuf;
+    char       *envvar = text_to_cstring(PG_GETARG_TEXT_PP(0));
+    char       *envval = text_to_cstring(PG_GETARG_TEXT_PP(1));

     if (!superuser())
         elog(ERROR, "must be superuser to change environment variables");

-    oldcontext = MemoryContextSwitchTo(TopMemoryContext);
-    envbuf = text_to_cstring((text *) PG_GETARG_POINTER(0));
-    MemoryContextSwitchTo(oldcontext);
-
-    if (putenv(envbuf) != 0)
+    if (setenv(envvar, envval, 1) != 0)
         elog(ERROR, "could not set environment variable: %m");

     PG_RETURN_VOID();
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 22d6abd367..95d4e826b1 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -348,6 +348,7 @@ sub GenerateFiles
         HAVE_RL_FILENAME_QUOTING_FUNCTION        => undef,
         HAVE_RL_RESET_SCREEN_SIZE                => undef,
         HAVE_SECURITY_PAM_APPL_H                 => undef,
+        HAVE_SETENV                              => undef,
         HAVE_SETPROCTITLE                        => undef,
         HAVE_SETPROCTITLE_FAST                   => undef,
         HAVE_SETSID                              => undef,

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Weird failure in explain.out with OpenBSD
Next
From: Andrey Borodin
Date:
Subject: Re: [HACKERS] Custom compression methods