Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7ca37fb

Browse files
tglsfdctglsfdc
committed
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
1 parent 62097a4 commit 7ca37fb

File tree

26 files changed

+234
-199
lines changed

26 files changed

+234
-199
lines changed

‎configure

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15959,12 +15959,29 @@ case $host_os in
1595915959
# Windows uses a specialised env handler
1596015960
mingw*)
1596115961

15962+
$as_echo "#define HAVE_SETENV 1" >>confdefs.h
15963+
15964+
1596215965
$as_echo "#define HAVE_UNSETENV 1" >>confdefs.h
1596315966

15967+
ac_cv_func_setenv=yes
1596415968
ac_cv_func_unsetenv=yes
1596515969
;;
1596615970
*)
15967-
ac_fn_c_check_func "$LINENO" "unsetenv" "ac_cv_func_unsetenv"
15971+
ac_fn_c_check_func "$LINENO" "setenv" "ac_cv_func_setenv"
15972+
if test "x$ac_cv_func_setenv" = xyes; then :
15973+
$as_echo "#define HAVE_SETENV 1" >>confdefs.h
15974+
15975+
else
15976+
case " $LIBOBJS " in
15977+
*" setenv.$ac_objext "* ) ;;
15978+
*) LIBOBJS="$LIBOBJS setenv.$ac_objext"
15979+
;;
15980+
esac
15981+
15982+
fi
15983+
15984+
ac_fn_c_check_func "$LINENO" "unsetenv" "ac_cv_func_unsetenv"
1596815985
if test "x$ac_cv_func_unsetenv" = xyes; then :
1596915986
$as_echo "#define HAVE_UNSETENV 1" >>confdefs.h
1597015987

‎configure.ac

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1757,11 +1757,13 @@ fi
17571757
case $host_os in
17581758
# Windows uses a specialised env handler
17591759
mingw*)
1760+
AC_DEFINE(HAVE_SETENV, 1, [Define to 1 because replacement version used.])
17601761
AC_DEFINE(HAVE_UNSETENV, 1, [Define to 1 because replacement version used.])
1762+
ac_cv_func_setenv=yes
17611763
ac_cv_func_unsetenv=yes
17621764
;;
17631765
*)
1764-
AC_REPLACE_FUNCS([unsetenv])
1766+
AC_REPLACE_FUNCS([setenv unsetenv])
17651767
;;
17661768
esac
17671769

‎contrib/dblink/input/paths.source

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
-- Initialization that requires path substitution.
22

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

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

1313
CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
14-
AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;
14+
AS $$SELECT setenv('PGSERVICEFILE', '@abs_srcdir@/' || $1)$$;

‎contrib/dblink/output/paths.source

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
-- Initialization that requires path substitution.
2-
CREATE FUNCTION putenv(text)
2+
CREATE FUNCTION setenv(text, text)
33
RETURNS void
4-
AS '@libdir@/regress@DLSUFFIX@', 'regress_putenv'
4+
AS '@libdir@/regress@DLSUFFIX@', 'regress_setenv'
55
LANGUAGE C STRICT;
66
CREATE FUNCTION wait_pid(int)
77
RETURNS void
88
AS '@libdir@/regress@DLSUFFIX@'
99
LANGUAGE C STRICT;
1010
CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
11-
AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;
11+
AS $$SELECT setenv('PGSERVICEFILE', '@abs_srcdir@/' || $1)$$;

‎src/backend/utils/adt/pg_locale.c

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -105,20 +105,6 @@ char *localized_full_months[12 + 1];
105105
static bool CurrentLocaleConvValid = false;
106106
static bool CurrentLCTimeValid = false;
107107

108-
/* Environment variable storage area */
109-
110-
#define LC_ENV_BUFSIZE (NAMEDATALEN + 20)
111-
112-
static char lc_collate_envbuf[LC_ENV_BUFSIZE];
113-
static char lc_ctype_envbuf[LC_ENV_BUFSIZE];
114-
115-
#ifdef LC_MESSAGES
116-
static char lc_messages_envbuf[LC_ENV_BUFSIZE];
117-
#endif
118-
static char lc_monetary_envbuf[LC_ENV_BUFSIZE];
119-
static char lc_numeric_envbuf[LC_ENV_BUFSIZE];
120-
static char lc_time_envbuf[LC_ENV_BUFSIZE];
121-
122108
/* Cache for collation-related knowledge */
123109

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

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

204189
/* copy setlocale() return value before callee invokes it again */
205190
strlcpy(save_lc_ctype, result, sizeof(save_lc_ctype));
@@ -216,16 +201,13 @@ pg_perm_setlocale(int category, const char *locale)
216201
{
217202
case LC_COLLATE:
218203
envvar = "LC_COLLATE";
219-
envbuf = lc_collate_envbuf;
220204
break;
221205
case LC_CTYPE:
222206
envvar = "LC_CTYPE";
223-
envbuf = lc_ctype_envbuf;
224207
break;
225208
#ifdef LC_MESSAGES
226209
case LC_MESSAGES:
227210
envvar = "LC_MESSAGES";
228-
envbuf = lc_messages_envbuf;
229211
#ifdef WIN32
230212
result = IsoLocaleName(locale);
231213
if (result == NULL)
@@ -236,26 +218,19 @@ pg_perm_setlocale(int category, const char *locale)
236218
#endif /* LC_MESSAGES */
237219
case LC_MONETARY:
238220
envvar = "LC_MONETARY";
239-
envbuf = lc_monetary_envbuf;
240221
break;
241222
case LC_NUMERIC:
242223
envvar = "LC_NUMERIC";
243-
envbuf = lc_numeric_envbuf;
244224
break;
245225
case LC_TIME:
246226
envvar = "LC_TIME";
247-
envbuf = lc_time_envbuf;
248227
break;
249228
default:
250229
elog(FATAL, "unrecognized LC category: %d", category);
251-
envvar = NULL; /* keep compiler quiet */
252-
envbuf = NULL;
253-
return NULL;
230+
return NULL; /* keep compiler quiet */
254231
}
255232

256-
snprintf(envbuf, LC_ENV_BUFSIZE - 1, "%s=%s", envvar, result);
257-
258-
if (putenv(envbuf))
233+
if (setenv(envvar, result, 1) != 0)
259234
return NULL;
260235

261236
return result;

‎src/bin/initdb/initdb.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2355,8 +2355,7 @@ check_need_password(const char *authmethodlocal, const char *authmethodhost)
23552355
void
23562356
setup_pgdata(void)
23572357
{
2358-
char *pgdata_get_env,
2359-
*pgdata_set_env;
2358+
char *pgdata_get_env;
23602359

23612360
if (!pg_data)
23622361
{
@@ -2386,8 +2385,11 @@ setup_pgdata(void)
23862385
* need quotes otherwise on Windows because paths there are most likely to
23872386
* have embedded spaces.
23882387
*/
2389-
pgdata_set_env = psprintf("PGDATA=%s", pg_data);
2390-
putenv(pgdata_set_env);
2388+
if (setenv("PGDATA", pg_data, 1) != 0)
2389+
{
2390+
pg_log_error("could not set environment");
2391+
exit(1);
2392+
}
23912393
}
23922394

23932395

‎src/bin/pg_ctl/pg_ctl.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -889,11 +889,10 @@ do_start(void)
889889
*/
890890
#ifndef WIN32
891891
{
892-
static char env_var[32];
892+
char env_var[32];
893893

894-
snprintf(env_var, sizeof(env_var), "PG_GRANDPARENT_PID=%d",
895-
(int) getppid());
896-
putenv(env_var);
894+
snprintf(env_var, sizeof(env_var), "%d", (int) getppid());
895+
setenv("PG_GRANDPARENT_PID", env_var, 1);
897896
}
898897
#endif
899898

@@ -2340,19 +2339,18 @@ main(int argc, char **argv)
23402339
case 'D':
23412340
{
23422341
char *pgdata_D;
2343-
char *env_var;
23442342

23452343
pgdata_D = pg_strdup(optarg);
23462344
canonicalize_path(pgdata_D);
2347-
env_var = psprintf("PGDATA=%s", pgdata_D);
2348-
putenv(env_var);
2345+
setenv("PGDATA", pgdata_D, 1);
23492346

23502347
/*
23512348
* We could pass PGDATA just in an environment
23522349
* variable but we do -D too for clearer postmaster
23532350
* 'ps' display
23542351
*/
23552352
pgdata_opt = psprintf("-D \"%s\" ", pgdata_D);
2353+
free(pgdata_D);
23562354
break;
23572355
}
23582356
case 'e':

‎src/bin/pg_upgrade/controldata.c

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -97,20 +97,20 @@ get_control_data(ClusterInfo *cluster, bool live_check)
9797
if (getenv("LC_MESSAGES"))
9898
lc_messages = pg_strdup(getenv("LC_MESSAGES"));
9999

100-
pg_putenv("LC_COLLATE", NULL);
101-
pg_putenv("LC_CTYPE", NULL);
102-
pg_putenv("LC_MONETARY", NULL);
103-
pg_putenv("LC_NUMERIC", NULL);
104-
pg_putenv("LC_TIME", NULL);
100+
unsetenv("LC_COLLATE");
101+
unsetenv("LC_CTYPE");
102+
unsetenv("LC_MONETARY");
103+
unsetenv("LC_NUMERIC");
104+
unsetenv("LC_TIME");
105105
#ifndef WIN32
106-
pg_putenv("LANG", NULL);
106+
unsetenv("LANG");
107107
#else
108108
/* On Windows the default locale may not be English, so force it */
109-
pg_putenv("LANG", "en");
109+
setenv("LANG", "en", 1);
110110
#endif
111-
pg_putenv("LANGUAGE", NULL);
112-
pg_putenv("LC_ALL", NULL);
113-
pg_putenv("LC_MESSAGES", "C");
111+
unsetenv("LANGUAGE");
112+
unsetenv("LC_ALL");
113+
setenv("LC_MESSAGES", "C", 1);
114114

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

492492
/*
493-
* Restore environment variables
493+
* Restore environment variables. Note all but LANG and LC_MESSAGES were
494+
* unset above.
494495
*/
495-
pg_putenv("LC_COLLATE", lc_collate);
496-
pg_putenv("LC_CTYPE", lc_ctype);
497-
pg_putenv("LC_MONETARY", lc_monetary);
498-
pg_putenv("LC_NUMERIC", lc_numeric);
499-
pg_putenv("LC_TIME", lc_time);
500-
pg_putenv("LANG", lang);
501-
pg_putenv("LANGUAGE", language);
502-
pg_putenv("LC_ALL", lc_all);
503-
pg_putenv("LC_MESSAGES", lc_messages);
496+
if (lc_collate)
497+
setenv("LC_COLLATE", lc_collate, 1);
498+
if (lc_ctype)
499+
setenv("LC_CTYPE", lc_ctype, 1);
500+
if (lc_monetary)
501+
setenv("LC_MONETARY", lc_monetary, 1);
502+
if (lc_numeric)
503+
setenv("LC_NUMERIC", lc_numeric, 1);
504+
if (lc_time)
505+
setenv("LC_TIME", lc_time, 1);
506+
if (lang)
507+
setenv("LANG", lang, 1);
508+
else
509+
unsetenv("LANG");
510+
if (language)
511+
setenv("LANGUAGE", language, 1);
512+
if (lc_all)
513+
setenv("LC_ALL", lc_all, 1);
514+
if (lc_messages)
515+
setenv("LC_MESSAGES", lc_messages, 1);
516+
else
517+
unsetenv("LC_MESSAGES");
504518

505519
pg_free(lc_collate);
506520
pg_free(lc_ctype);

‎src/bin/pg_upgrade/option.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ parseCommandLine(int argc, char *argv[])
193193
* Push the user name into the environment so pre-9.1
194194
* pg_ctl/libpq uses it.
195195
*/
196-
pg_putenv("PGUSER", os_info.user);
196+
setenv("PGUSER", os_info.user, 1);
197197
break;
198198

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

248-
pg_putenv("PGOPTIONS", pgoptions);
248+
setenv("PGOPTIONS", pgoptions, 1);
249249
pfree(pgoptions);
250250
}
251251
else
252-
pg_putenv("PGOPTIONS", FIX_DEFAULT_READ_ONLY);
252+
setenv("PGOPTIONS", FIX_DEFAULT_READ_ONLY, 1);
253253

254254
/* Get values from env if not already set */
255255
check_required_directory(&old_cluster.bindir, "PGBINOLD", false,

‎src/bin/pg_upgrade/pg_upgrade.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,6 @@ void end_progress_output(void);
436436
void prep_status(const char *fmt,...) pg_attribute_printf(1, 2);
437437
void check_ok(void);
438438
unsigned int str2uint(const char *str);
439-
void pg_putenv(const char *var, const char *val);
440439

441440

442441
/* version.c */

‎src/bin/pg_upgrade/util.c

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -241,39 +241,3 @@ str2uint(const char *str)
241241
{
242242
return strtoul(str, NULL, 10);
243243
}
244-
245-
246-
/*
247-
* pg_putenv()
248-
*
249-
* This is like putenv(), but takes two arguments.
250-
* It also does unsetenv() if val is NULL.
251-
*/
252-
void
253-
pg_putenv(const char *var, const char *val)
254-
{
255-
if (val)
256-
{
257-
#ifndef WIN32
258-
char *envstr;
259-
260-
envstr = psprintf("%s=%s", var, val);
261-
putenv(envstr);
262-
263-
/*
264-
* Do not free envstr because it becomes part of the environment on
265-
* some operating systems. See port/unsetenv.c::unsetenv.
266-
*/
267-
#else
268-
SetEnvironmentVariableA(var, val);
269-
#endif
270-
}
271-
else
272-
{
273-
#ifndef WIN32
274-
unsetenv(var);
275-
#else
276-
SetEnvironmentVariableA(var, "");
277-
#endif
278-
}
279-
}

‎src/bin/psql/command.c

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2296,17 +2296,8 @@ exec_command_setenv(PsqlScanState scan_state, bool active_branch,
22962296
else
22972297
{
22982298
/* Set variable to the value of the next argument */
2299-
char *newval;
2300-
2301-
newval = psprintf("%s=%s", envvar, envval);
2302-
putenv(newval);
2299+
setenv(envvar, envval, 1);
23032300
success = true;
2304-
2305-
/*
2306-
* Do not free newval here, it will screw up the environment if
2307-
* you do. See putenv man page for details. That means we leak a
2308-
* bit of memory here, but not enough to worry about.
2309-
*/
23102301
}
23112302
free(envvar);
23122303
free(envval);

0 commit comments

Comments
 (0)