From 7ca37fb0406bc2cbbd864a2ffdbdb4479e338c0c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 30 Dec 2020 12:55:59 -0500 Subject: 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 --- src/test/isolation/isolation_main.c | 6 ++--- src/test/regress/pg_regress.c | 45 ++++++++++++++----------------------- src/test/regress/pg_regress_main.c | 6 ++--- src/test/regress/regress.c | 14 +++++------- 4 files changed, 28 insertions(+), 43 deletions(-) (limited to 'src/test') 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 @@ -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 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(); -- cgit v1.2.3