Skip to content

Commit abd3f43

Browse files
tglsfdctglsfdc
committed
Fix syslog bug: if any messages are emitted to write_syslog before
the facility has been set, the facility gets set to LOCAL0 and cannot be changed later. This seems reasonably plausible to happen, particularly at higher debug log levels, though I am not certain it explains Han Holl's recent report. Easiest fix is to teach the code how to change the value on-the-fly, which is nicer anyway. I made the settings PGC_SIGHUP to conform with log_destination.
1 parent f620098 commit abd3f43

File tree

4 files changed

+110
-73
lines changed

4 files changed

+110
-73
lines changed

‎doc/src/sgml/config.sgml

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.28 2005/10/13 22:55:19 momjian Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.29 2005/10/14 20:53:55 tgl Exp $
33
-->
44
<chapter Id="runtime-config">
55
<title>Run-time Configuration</title>
@@ -2254,7 +2254,8 @@ SELECT * FROM parent WHERE key = 2400;
22542254
the default is <literal>LOCAL0</>. See also the
22552255
documentation of your system's
22562256
<application>syslog</application> daemon.
2257-
This option can only be set at server start.
2257+
This option can only be set at server start or in the
2258+
<filename>postgresql.conf</filename> configuration file.
22582259
</para>
22592260
</listitem>
22602261
</varlistentry>
@@ -2271,7 +2272,8 @@ SELECT * FROM parent WHERE key = 2400;
22712272
<productname>PostgreSQL</productname> messages in
22722273
<application>syslog</application> logs. The default is
22732274
<literal>postgres</literal>.
2274-
This option can only be set at server start.
2275+
This option can only be set at server start or in the
2276+
<filename>postgresql.conf</filename> configuration file.
22752277
</para>
22762278
</listitem>
22772279
</varlistentry>
@@ -2581,8 +2583,8 @@ SELECT * FROM parent WHERE key = 2400;
25812583
<varname>log_statement</> to be logged. When using this option,
25822584
if you are not using <application>syslog</>, it is recommended
25832585
that you log the PID or session ID using <varname>log_line_prefix</>
2584-
so that you can link the statement to the
2585-
duration using the process ID or session ID. The default is
2586+
so that you can link the statement message to the later
2587+
duration message using the process ID or session ID. The default is
25862588
<literal>off</>. Only superusers can change this setting.
25872589
</para>
25882590
</listitem>

‎src/backend/utils/error/elog.c

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*
4343
*
4444
* IDENTIFICATION
45-
* $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.163 2005/10/14 16:41:02 tgl Exp $
45+
* $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.164 2005/10/14 20:53:56 tgl Exp $
4646
*
4747
*-------------------------------------------------------------------------
4848
*/
@@ -80,8 +80,9 @@ char *Log_line_prefix = NULL; /* format for extra log line info */
8080
int Log_destination = LOG_DESTINATION_STDERR;
8181

8282
#ifdef HAVE_SYSLOG
83-
char *Syslog_facility; /* openlog() parameters */
84-
char *Syslog_ident;
83+
static bool openlog_done = false;
84+
static char *syslog_ident = NULL;
85+
static int syslog_facility = LOG_LOCAL0;
8586

8687
static void write_syslog(int level, const char *line);
8788
#endif
@@ -1148,6 +1149,39 @@ DebugFileOpen(void)
11481149

11491150
#ifdef HAVE_SYSLOG
11501151

1152+
/*
1153+
* Set or update the parameters for syslog logging
1154+
*/
1155+
void
1156+
set_syslog_parameters(const char *ident, int facility)
1157+
{
1158+
/*
1159+
* guc.c is likely to call us repeatedly with same parameters, so
1160+
* don't thrash the syslog connection unnecessarily. Also, we do not
1161+
* re-open the connection until needed, since this routine will get called
1162+
* whether or not Log_destination actually mentions syslog.
1163+
*
1164+
* Note that we make our own copy of the ident string rather than relying
1165+
* on guc.c's. This may be overly paranoid, but it ensures that we cannot
1166+
* accidentally free a string that syslog is still using.
1167+
*/
1168+
if (syslog_ident == NULL || strcmp(syslog_ident, ident) != 0 ||
1169+
syslog_facility != facility)
1170+
{
1171+
if (openlog_done)
1172+
{
1173+
closelog();
1174+
openlog_done = false;
1175+
}
1176+
if (syslog_ident)
1177+
free(syslog_ident);
1178+
syslog_ident = strdup(ident);
1179+
/* if the strdup fails, we will cope in write_syslog() */
1180+
syslog_facility = facility;
1181+
}
1182+
}
1183+
1184+
11511185
#ifndef PG_SYSLOG_LIMIT
11521186
#define PG_SYSLOG_LIMIT 128
11531187
#endif
@@ -1158,46 +1192,16 @@ DebugFileOpen(void)
11581192
static void
11591193
write_syslog(int level, const char *line)
11601194
{
1161-
static bool openlog_done = false;
11621195
static unsigned long seq = 0;
11631196

11641197
int len;
11651198

1199+
/* Open syslog connection if not done yet */
11661200
if (!openlog_done)
11671201
{
1168-
int syslog_fac;
1169-
char *syslog_ident;
1170-
1171-
if (pg_strcasecmp(Syslog_facility, "LOCAL0") == 0)
1172-
syslog_fac = LOG_LOCAL0;
1173-
else if (pg_strcasecmp(Syslog_facility, "LOCAL1") == 0)
1174-
syslog_fac = LOG_LOCAL1;
1175-
else if (pg_strcasecmp(Syslog_facility, "LOCAL2") == 0)
1176-
syslog_fac = LOG_LOCAL2;
1177-
else if (pg_strcasecmp(Syslog_facility, "LOCAL3") == 0)
1178-
syslog_fac = LOG_LOCAL3;
1179-
else if (pg_strcasecmp(Syslog_facility, "LOCAL4") == 0)
1180-
syslog_fac = LOG_LOCAL4;
1181-
else if (pg_strcasecmp(Syslog_facility, "LOCAL5") == 0)
1182-
syslog_fac = LOG_LOCAL5;
1183-
else if (pg_strcasecmp(Syslog_facility, "LOCAL6") == 0)
1184-
syslog_fac = LOG_LOCAL6;
1185-
else if (pg_strcasecmp(Syslog_facility, "LOCAL7") == 0)
1186-
syslog_fac = LOG_LOCAL7;
1187-
else
1188-
syslog_fac = LOG_LOCAL0;
1189-
/*
1190-
* openlog() usually just stores the passed char pointer as-is,
1191-
* so we must give it a string that will be unchanged for the life of
1192-
* the process. The Syslog_ident GUC variable does not meet this
1193-
* requirement, so strdup() it. This isn't a memory leak because
1194-
* this code is executed at most once per process.
1195-
*/
1196-
syslog_ident = strdup(Syslog_ident);
1197-
if (syslog_ident == NULL) /* out of memory already!? */
1198-
syslog_ident = "postgres";
1199-
1200-
openlog(syslog_ident, LOG_PID | LOG_NDELAY | LOG_NOWAIT, syslog_fac);
1202+
openlog(syslog_ident ? syslog_ident : "postgres",
1203+
LOG_PID | LOG_NDELAY | LOG_NOWAIT,
1204+
syslog_facility);
12011205
openlog_done = true;
12021206
}
12031207

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

Lines changed: 58 additions & 30 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.291 2005/10/08 20:08:19 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.292 2005/10/14 20:53:56 tgl Exp $
1414
*
1515
*--------------------------------------------------------------------
1616
*/
@@ -21,6 +21,9 @@
2121
#include <limits.h>
2222
#include <unistd.h>
2323
#include <sys/stat.h>
24+
#ifdef HAVE_SYSLOG
25+
#include <syslog.h>
26+
#endif
2427

2528
#include "utils/guc.h"
2629
#include "utils/guc_tables.h"
@@ -100,10 +103,11 @@ static const char *assign_log_destination(const char *value,
100103
bool doit, GucSource source);
101104

102105
#ifdef HAVE_SYSLOG
103-
extern char *Syslog_facility;
104-
extern char *Syslog_ident;
106+
static int syslog_facility = LOG_LOCAL0;
105107

106-
static const char *assign_facility(const char *facility,
108+
static const char *assign_syslog_facility(const char *facility,
109+
bool doit, GucSource source);
110+
static const char *assign_syslog_ident(const char *ident,
107111
bool doit, GucSource source);
108112
#endif
109113

@@ -192,6 +196,8 @@ static char *log_error_verbosity_str;
192196
static char *log_statement_str;
193197
static char *log_min_error_statement_str;
194198
static char *log_destination_string;
199+
static char *syslog_facility_str;
200+
static char *syslog_ident_str;
195201
static bool phony_autocommit;
196202
static bool session_auth_is_superuser;
197203
static double phony_random_seed;
@@ -1964,22 +1970,22 @@ static struct config_string ConfigureNamesString[] =
19641970

19651971
#ifdef HAVE_SYSLOG
19661972
{
1967-
{"syslog_facility", PGC_POSTMASTER, LOGGING_WHERE,
1973+
{"syslog_facility", PGC_SIGHUP, LOGGING_WHERE,
19681974
gettext_noop("Sets the syslog \"facility\" to be used when syslog enabled."),
19691975
gettext_noop("Valid values are LOCAL0, LOCAL1, LOCAL2, LOCAL3, "
19701976
"LOCAL4, LOCAL5, LOCAL6, LOCAL7.")
19711977
},
1972-
&Syslog_facility,
1973-
"LOCAL0", assign_facility, NULL
1978+
&syslog_facility_str,
1979+
"LOCAL0", assign_syslog_facility, NULL
19741980
},
19751981
{
1976-
{"syslog_ident", PGC_POSTMASTER, LOGGING_WHERE,
1977-
gettext_noop("Sets the program name used to identify PostgreSQL messages "
1978-
"in syslog."),
1982+
{"syslog_ident", PGC_SIGHUP, LOGGING_WHERE,
1983+
gettext_noop("Sets the program name used to identify PostgreSQL "
1984+
"messages in syslog."),
19791985
NULL
19801986
},
1981-
&Syslog_ident,
1982-
"postgres", NULL, NULL
1987+
&syslog_ident_str,
1988+
"postgres", assign_syslog_ident, NULL
19831989
},
19841990
#endif
19851991

@@ -5552,27 +5558,49 @@ assign_log_destination(const char *value, bool doit, GucSource source)
55525558
#ifdef HAVE_SYSLOG
55535559

55545560
static const char *
5555-
assign_facility(const char *facility, bool doit, GucSource source)
5561+
assign_syslog_facility(const char *facility, bool doit, GucSource source)
55565562
{
5563+
int syslog_fac;
5564+
55575565
if (pg_strcasecmp(facility, "LOCAL0") == 0)
5558-
return facility;
5559-
if (pg_strcasecmp(facility, "LOCAL1") == 0)
5560-
return facility;
5561-
if (pg_strcasecmp(facility, "LOCAL2") == 0)
5562-
return facility;
5563-
if (pg_strcasecmp(facility, "LOCAL3") == 0)
5564-
return facility;
5565-
if (pg_strcasecmp(facility, "LOCAL4") == 0)
5566-
return facility;
5567-
if (pg_strcasecmp(facility, "LOCAL5") == 0)
5568-
return facility;
5569-
if (pg_strcasecmp(facility, "LOCAL6") == 0)
5570-
return facility;
5571-
if (pg_strcasecmp(facility, "LOCAL7") == 0)
5572-
return facility;
5573-
return NULL;
5566+
syslog_fac = LOG_LOCAL0;
5567+
else if (pg_strcasecmp(facility, "LOCAL1") == 0)
5568+
syslog_fac = LOG_LOCAL1;
5569+
else if (pg_strcasecmp(facility, "LOCAL2") == 0)
5570+
syslog_fac = LOG_LOCAL2;
5571+
else if (pg_strcasecmp(facility, "LOCAL3") == 0)
5572+
syslog_fac = LOG_LOCAL3;
5573+
else if (pg_strcasecmp(facility, "LOCAL4") == 0)
5574+
syslog_fac = LOG_LOCAL4;
5575+
else if (pg_strcasecmp(facility, "LOCAL5") == 0)
5576+
syslog_fac = LOG_LOCAL5;
5577+
else if (pg_strcasecmp(facility, "LOCAL6") == 0)
5578+
syslog_fac = LOG_LOCAL6;
5579+
else if (pg_strcasecmp(facility, "LOCAL7") == 0)
5580+
syslog_fac = LOG_LOCAL7;
5581+
else
5582+
return NULL; /* reject */
5583+
5584+
if (doit)
5585+
{
5586+
syslog_facility = syslog_fac;
5587+
set_syslog_parameters(syslog_ident_str ? syslog_ident_str : "postgres",
5588+
syslog_facility);
5589+
}
5590+
5591+
return facility;
55745592
}
5575-
#endif
5593+
5594+
static const char *
5595+
assign_syslog_ident(const char *ident, bool doit, GucSource source)
5596+
{
5597+
if (doit)
5598+
set_syslog_parameters(ident, syslog_facility);
5599+
5600+
return ident;
5601+
}
5602+
5603+
#endif /* HAVE_SYSLOG */
55765604

55775605

55785606
static const char *

‎src/include/utils/elog.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/utils/elog.h,v 1.79 2005/06/10 16:23:10 neilc Exp $
10+
* $PostgreSQL: pgsql/src/include/utils/elog.h,v 1.80 2005/10/14 20:53:56 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -283,6 +283,9 @@ extern int Log_destination;
283283
/* Other exported functions */
284284
extern void DebugFileOpen(void);
285285
extern char *unpack_sql_state(int sql_state);
286+
#ifdef HAVE_SYSLOG
287+
extern void set_syslog_parameters(const char *ident, int facility);
288+
#endif
286289

287290
/*
288291
* Write errors to stderr (or by equal means when stderr is

0 commit comments

Comments
 (0)