Skip to content

Commit 54d60bb

Browse files
tglsfdctglsfdc
committed
Fix a couple of issues in recent patch to print updates to postgresql.conf
settings: avoid calling superuser() in contexts where it's not defined, don't leak the transient copies of GetConfigOption output, and avoid the whole exercise in postmaster child processes. I found that actually no current caller of GetConfigOption has any use for its internal check of GUC_SUPERUSER_ONLY. But rather than just remove that entirely, it seemed better to add a parameter indicating whether to enforce the check. Per report from Simon and subsequent testing.
1 parent 66a8417 commit 54d60bb

File tree

4 files changed

+26
-15
lines changed

4 files changed

+26
-15
lines changed

‎src/backend/utils/misc/guc-file.l

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*
55
* Copyright (c) 2000-2009, PostgreSQL Global Development Group
66
*
7-
* $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.61 2009/09/17 21:15:18 petere Exp $
7+
* $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.62 2009/10/03 18:04:57 tgl Exp $
88
*/
99

1010
%{
@@ -312,21 +312,26 @@ ProcessConfigFile(GucContext context)
312312
/* If we got here all the options checked out okay, so apply them. */
313313
for (item = head; item; item = item->next)
314314
{
315-
char *pre_value = NULL;
315+
char *pre_value = NULL;
316316

317-
if (context == PGC_SIGHUP)
318-
pre_value = pstrdup(GetConfigOption(item->name));
317+
/* In SIGHUP cases in the postmaster, report changes */
318+
if (context == PGC_SIGHUP && !IsUnderPostmaster)
319+
pre_value = pstrdup(GetConfigOption(item->name, false));
319320

320321
if (set_config_option(item->name, item->value, context,
321322
PGC_S_FILE, GUC_ACTION_SET, true))
322323
{
323-
if (pre_value && strcmp(pre_value, GetConfigOption(item->name)) != 0)
324+
set_config_sourcefile(item->name, item->filename,
325+
item->sourceline);
326+
if (pre_value &&
327+
strcmp(pre_value, GetConfigOption(item->name, false)) != 0)
324328
ereport(elevel,
325329
(errmsg("parameter \"%s\" changed to \"%s\"",
326330
item->name, item->value)));
327-
set_config_sourcefile(item->name, item->filename,
328-
item->sourceline);
329331
}
332+
333+
if (pre_value)
334+
pfree(pre_value);
330335
}
331336

332337
/* Remember when we last successfully loaded the config file. */

‎src/backend/utils/misc/guc.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Written by Peter Eisentraut <peter_e@gmx.net>.
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.519 2009/09/22 23:43:38 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.520 2009/10/03 18:04:57 tgl Exp $
1414
*
1515
*--------------------------------------------------------------------
1616
*/
@@ -5197,11 +5197,15 @@ SetConfigOption(const char *name, const char *value,
51975197
* Fetch the current value of the option `name'. If the option doesn't exist,
51985198
* throw an ereport and don't return.
51995199
*
5200+
* If restrict_superuser is true, we also enforce that only superusers can
5201+
* see GUC_SUPERUSER_ONLY variables. This should only be passed as true
5202+
* in user-driven calls.
5203+
*
52005204
* The string is *not* allocated for modification and is really only
52015205
* valid until the next call to configuration related functions.
52025206
*/
52035207
const char *
5204-
GetConfigOption(const char *name)
5208+
GetConfigOption(const char *name, bool restrict_superuser)
52055209
{
52065210
struct config_generic *record;
52075211
static char buffer[256];
@@ -5211,7 +5215,9 @@ GetConfigOption(const char *name)
52115215
ereport(ERROR,
52125216
(errcode(ERRCODE_UNDEFINED_OBJECT),
52135217
errmsg("unrecognized configuration parameter \"%s\"", name)));
5214-
if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser())
5218+
if (restrict_superuser &&
5219+
(record->flags & GUC_SUPERUSER_ONLY) &&
5220+
!superuser())
52155221
ereport(ERROR,
52165222
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
52175223
errmsg("must be superuser to examine \"%s\"", name)));

‎src/include/utils/guc.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Copyright (c) 2000-2009, PostgreSQL Global Development Group
88
* Written by Peter Eisentraut <peter_e@gmx.net>.
99
*
10-
* $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.105 2009/09/22 23:43:41 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.106 2009/10/03 18:04:57 tgl Exp $
1111
*--------------------------------------------------------------------
1212
*/
1313
#ifndef GUC_H
@@ -249,7 +249,7 @@ extern void DefineCustomEnumVariable(
249249

250250
extern void EmitWarningsOnPlaceholders(const char *className);
251251

252-
extern const char *GetConfigOption(const char *name);
252+
extern const char *GetConfigOption(const char *name, bool restrict_superuser);
253253
extern const char *GetConfigOptionResetString(const char *name);
254254
extern void ProcessConfigFile(GucContext context);
255255
extern void InitializeGUCOptions(void);

‎src/timezone/pgtz.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
77
*
88
* IDENTIFICATION
9-
* $PostgreSQL: pgsql/src/timezone/pgtz.c,v 1.63 2009/06/11 14:49:15 momjian Exp $
9+
* $PostgreSQL: pgsql/src/timezone/pgtz.c,v 1.64 2009/10/03 18:04:57 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -1366,7 +1366,7 @@ pg_timezone_initialize(void)
13661366
pg_tz *def_tz = NULL;
13671367

13681368
/* Do we need to try to figure the session timezone? */
1369-
if (pg_strcasecmp(GetConfigOption("timezone"), "UNKNOWN") == 0)
1369+
if (pg_strcasecmp(GetConfigOption("timezone", false), "UNKNOWN") == 0)
13701370
{
13711371
/* Select setting */
13721372
def_tz = select_default_timezone();
@@ -1377,7 +1377,7 @@ pg_timezone_initialize(void)
13771377
}
13781378

13791379
/* What about the log timezone? */
1380-
if (pg_strcasecmp(GetConfigOption("log_timezone"), "UNKNOWN") == 0)
1380+
if (pg_strcasecmp(GetConfigOption("log_timezone", false), "UNKNOWN") == 0)
13811381
{
13821382
/* Select setting, but don't duplicate work */
13831383
if (!def_tz)

0 commit comments

Comments
 (0)