Use setenv() in preference to putenv().

Since at least 2001 we've used putenv() and avoided setenv(), on the
grounds that the latter was unportable and not in POSIX.  However,
POSIX added it that same year, and by now the situation has reversed:
setenv() is probably more portable than putenv(), since POSIX now
treats the latter as not being a core function.  And setenv() has
cleaner semantics too.  So, let's reverse that old policy.

This commit adds a simple src/port/ implementation of setenv() for
any stragglers (we have one in the buildfarm, but I'd not be surprised
if that code is never used in the field).  More importantly, extend
win32env.c to also support setenv().  Then, replace usages of putenv()
with setenv(), and get rid of some ad-hoc implementations of setenv()
wannabees.

Also, adjust our src/port/ implementation of unsetenv() to follow the
POSIX spec that it returns an error indicator, rather than returning
void as per the ancient BSD convention.  I don't feel a need to make
all the call sites check for errors, but the portability stub ought
to match real-world practice.

Discussion: https://postgr.es/m/2065122.1609212051@sss.pgh.pa.us

4 files changed, 28 insertions, 43 deletions

diff --git a/src/test/isolation/isolation_main.c b/src/test/isolation/isolation_main.c
index 50916b00dca..a6a64e7ec52 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 23d7d0beb2e..866bc8c4704 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -725,18 +725,6 @@ get_expectfile(const char *testname, const char *file)
 }
 
 /*
- * 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
  */
 static void
@@ -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 dd8ad245648..5e503efa4a7 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 09bc42a8c0f..b8b3af4e956 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();