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

7 files changed, 49 insertions, 81 deletions

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index f994c4216bc..0865f73ee0b 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("could not set environment");
+		exit(1);
+	}
 }
 
 
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index fc07f1aba6e..fcdc0213e48 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 39bcaa8fe1a..7eb56e7a293 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 548d648e8c4..5b566c14ef4 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 ee70243c2e9..1842556274f 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 a16c794261b..9c9ba29124e 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 38b588882d1..c545341cddd 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);