Thread: Let's start using setenv()
Back in 2001 we decided to prefer putenv() over setenv() because the
latter wasn't in POSIX (SUSv2) at the time. That decision has been
overtaken by events: more recent editions of POSIX not only provide
setenv() but recommend it over putenv(). So I think it's time to
change our policy and prefer setenv(). We've had previous discussions
about that but nobody did the legwork yet.
The attached patch provides the infrastructure to allow using setenv().
I added a src/port/ implementation of setenv() for any machines that
haven't caught up with POSIX lately. (I've tested that code on my old
dinosaur gaur, and I would not be surprised if that is the only machine
anywhere that ever runs it. But I could be wrong.) I also extended
win32env.c to support setenv().
I haven't made any serious effort to expunge all uses of putenv() in this
version of the patch; I just wanted to exercise setenv() in both backend
and frontend. Seeing that POSIX considers putenv() to be semi-deprecated,
maybe we should try to eliminate all calls outside the (un)setenv
implementations, but first it'd be good to see if win32env.c works.
I also changed our unsetenv() emulations to make them return an int
error indicator, as per POSIX. I have no desire to change the call
sites to check for errors, but it seemed like our compatibility stubs
should be compatible with the spec. (Note: I think that unsetenv()
did return void in old BSD versions, before POSIX standardized it.
So that might be a real reason not to change the callers.)
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/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..148a0984ea 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))
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/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/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/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,
On Tue, Dec 29, 2020 at 4:21 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Back in 2001 we decided to prefer putenv() over setenv() because the > latter wasn't in POSIX (SUSv2) at the time. That decision has been > overtaken by events: more recent editions of POSIX not only provide > setenv() but recommend it over putenv(). So I think it's time to > change our policy and prefer setenv(). We've had previous discussions > about that but nobody did the legwork yet. > > The attached patch provides the infrastructure to allow using setenv(). > I added a src/port/ implementation of setenv() for any machines that > haven't caught up with POSIX lately. (I've tested that code on my old > dinosaur gaur, and I would not be surprised if that is the only machine > anywhere that ever runs it. But I could be wrong.) I also extended > win32env.c to support setenv(). +1, nice modernisation. > I also changed our unsetenv() emulations to make them return an int > error indicator, as per POSIX. I have no desire to change the call > sites to check for errors, but it seemed like our compatibility stubs > should be compatible with the spec. (Note: I think that unsetenv() > did return void in old BSD versions, before POSIX standardized it. > So that might be a real reason not to change the callers.) +1 for conformance. I suppose it's out of spec that unsetenv() can return -1 with errno = ENOMEM (from malloc()), but that hardly matters. Hmm... could a static buffer be used for that clobbering trick? For the note in parens: it looks like the 3 main BSDs all switched from the historical void function to the POSIX one 12-18 years ago[1][2][3], so I wouldn't worry about that. Glibc made a similar change 19 years ago. [1] https://github.com/NetBSD/src/commit/13dee93fb3a93189a74a3705a5f7cd1c6b290125 [2] https://github.com/openbsd/src/commit/471b62eeaa7f3a18c0aa98b5d605e5cec1625b62 [3] https://github.com/freebsd/freebsd/commit/196b6346ba4e13a3f7679e2de3317b6aa65983df
Thomas Munro <thomas.munro@gmail.com> writes:
> On Tue, Dec 29, 2020 at 4:21 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I also changed our unsetenv() emulations to make them return an int
>> error indicator, as per POSIX. I have no desire to change the call
>> sites to check for errors, but it seemed like our compatibility stubs
>> should be compatible with the spec. (Note: I think that unsetenv()
>> did return void in old BSD versions, before POSIX standardized it.
>> So that might be a real reason not to change the callers.)
> +1 for conformance. I suppose it's out of spec that unsetenv() can
> return -1 with errno = ENOMEM (from malloc()), but that hardly
> matters. Hmm... could a static buffer be used for that clobbering
> trick?
Hm, hadn't thought about that particularly. I did think about
hacking setenv.c to try to reduce memory leakage from repeated
sets of the same variable (basically, try to overwrite the existing
environ member whenever the new value is no longer than the old).
But on the whole I think it'd all be wasted effort. Neither setenv.c
nor unsetenv.c will ever be used on any machine that anyone would care
about performance of. If I were willing to retire gaur I'd vote for
just nuking both of them.
> For the note in parens: it looks like the 3 main BSDs all switched
> from the historical void function to the POSIX one 12-18 years
> ago[1][2][3], so I wouldn't worry about that. Glibc made a similar
> change 19 years ago.
Ah, thanks for the research. I'd found the glibc change from references
in current Linux man pages, but I didn't run down the BSD history.
(My other pet dinosaur prairiedog still has void unsetenv(), btw,
presumably because macOS is based on turn-of-the-century NetBSD.
Modern macOS does follow POSIX here; not sure when they changed.)
regards, tom lane
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,
On Wed, Dec 30, 2020 at 5:45 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.
+ 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;
}
Wouldn't it be better to do "cannot set environment: %m" or similar,
and let ENOMEM do its thing if that be the cause? It's true that
POSIX only allows EINVAL or ENOMEM and we can see no reason for EINVAL
here, but still it seems unnecessary to assume.
- 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);
Did you want to drop the canonicalize_path() logic here and nearby?
Thomas Munro <thomas.munro@gmail.com> writes:
> On Wed, Dec 30, 2020 at 5:45 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> + /* We assume the error must be "out of memory" */
> + ereport(LOG,
> + (errcode(ERRCODE_OUT_OF_MEMORY),
> + errmsg("out of memory")));
> Wouldn't it be better to do "cannot set environment: %m" or similar,
> and let ENOMEM do its thing if that be the cause?
That's no problem as far as the error text goes, but we lack a way to
choose a relevant errcode(). I guess I could fix it for ENOMEM
specifically, along the lines of
errcode(errno == ENOMEM ? ERRCODE_OUT_OF_MEMORY :
ERRCODE_INTERNAL_ERROR)
This is a bit trickier than it looks though, because code within an
ereport macro isn't really supposed to rely on errno still being
the same as at entry. Is it worth inventing an errcode_for_errno()
helper routine, which could look at the stashed errno? Or maybe
extending/abusing errcode_for_file_access() so it recognizes ENOMEM?
Or we could just use ERRCODE_OUT_OF_MEMORY, without being too picky
about whether that's accurate.
> Did you want to drop the canonicalize_path() logic here and nearby?
Yeah, because the results of get_locale_path et al are already
canonicalized, so those canonicalize_path calls are redundant.
Thanks for looking at the patch!
regards, tom lane