Обсуждение: GUC option log_pid is not checked [Fwd: Bug#149675: Fix]
The GUC option log_pid is not checked; LOG_PID is used by default,
contrary to the documentation. This patch fixes it.
-----Forwarded Message-----
From: Ymir <ymir@wolfheart.ro>
To: 149675@bugs.debian.org
Subject: Bug#149675: Fix
Date: 11 Jun 2002 18:28:43 +0300
As I said, I have a fix for it, a simple patch that solves it. (I've tested
it and it works, I can't see how it can be buggy seeing how simple it is)
----
diff -ruN postgresql-7.2.1/src/backend/utils/error/elog.c postgresql-7.2.1-patched/src/backend/utils/error/elog.c
--- postgresql-7.2.1/src/backend/utils/error/elog.c Mon Nov 5 19:46:30 2001
+++ postgresql-7.2.1-patched/src/backend/utils/error/elog.c Mon Jun 10 13:58:02 2002
@@ -563,6 +563,7 @@
{
static bool openlog_done = false;
static unsigned long seq = 0;
+ static int log_options = LOG_NDELAY;
static int syslog_fac = LOG_LOCAL0;
int len = strlen(line);
@@ -588,7 +589,16 @@
syslog_fac = LOG_LOCAL6;
if (strcasecmp(Syslog_facility, "LOCAL7") == 0)
syslog_fac = LOG_LOCAL7;
- openlog(Syslog_ident, LOG_PID | LOG_NDELAY, syslog_fac);
+/*
+ * We never checked for the Log_pid configure option when logging to syslog.
+ * The fix is pretty self-explaining, we just check for it, set the
+ * appropriate value for log_options, with, or without LOG_PID.
+ * <ymir@wolfheart.ro>
+ */
+
+ if (Log_pid)
+ log_options = LOG_PID | LOG_NDELAY;
+ openlog(Syslog_ident, log_options, syslog_fac);
openlog_done = true;
}
-----End of Forwarded Message-----
--
Oliver Elphick Oliver.Elphick@lfix.co.uk
Isle of Wight http://www.lfix.co.uk/oliver
GPG: 1024D/3E1D0C1C: CA12 09E0 E8D5 8870 5839 932A 614D 4C34 3E1D 0C1C
"Then said Jesus, Father, forgive them; for they know
not what they do..." Luke 23:34
Вложения
Oliver Elphick <olly@lfix.co.uk> writes:
> The GUC option log_pid is not checked; LOG_PID is used by default,
> contrary to the documentation. This patch fixes it.
No it doesn't (at least not unless you are also going to propose closing
and reopening syslog at SIGHUP to deal with changes in log_pid setting).
I'm not convinced that there's anything wrong with the code anyway.
Do you expect log_timestamp to control whether timestamps are added to
syslog entries? If so you are out of luck. Perhaps we should just
tweak the documentation to make it clearer that log_pid and
log_timestamp only control the format of non-syslog logging.
regards, tom lane
On Wed, 2002-06-12 at 15:12, Tom Lane wrote:
> Oliver Elphick <olly@lfix.co.uk> writes:
> > The GUC option log_pid is not checked; LOG_PID is used by default,
> > contrary to the documentation. This patch fixes it.
>
> No it doesn't (at least not unless you are also going to propose closing
> and reopening syslog at SIGHUP to deal with changes in log_pid setting).
... partially fixes it :-(
> I'm not convinced that there's anything wrong with the code anyway.
> Do you expect log_timestamp to control whether timestamps are added to
> syslog entries? If so you are out of luck. Perhaps we should just
syslog() recognises LOG_PID as a flag to request logging of PIDs; there
is no equivalent for the timestamp, of course.
> tweak the documentation to make it clearer that log_pid and
> log_timestamp only control the format of non-syslog logging.
OK.
If that is its real purpose, I think both I and the original submitter
misunderstood the intention of this parameter, which certainly doesn't
distinguish between logs via syslog and messages to the log file:
LOG_PID (boolean)
Prefixes each server log message with the process ID of the backend
process. This is useful to sort out which messages pertain to which
connection. The default is off.
Perhaps that should read:
LOG_PID (boolean)
Prefixes each server message in the logfile with the process ID of
the backend process. This is useful to sort out which messages
pertain to which connection. The default is off. This parameter
does not affect messages logged via syslog(), which always contain
the process ID.
I don't actually consider it a good thing to omit the PID in syslog, so
I am happy that this change is not necessary, provided that the
distinction between syslog and logfile is made clear.
--
Oliver Elphick Oliver.Elphick@lfix.co.uk
Isle of Wight http://www.lfix.co.uk/oliver
GPG: 1024D/3E1D0C1C: CA12 09E0 E8D5 8870 5839 932A 614D 4C34 3E1D 0C1C
"And he said unto Jesus, Lord, remember me when thou
comest into thy kingdom. And Jesus said unto him,
Verily I say unto thee, To day shalt thou be with me
in paradise." Luke 23:42,43
Вложения
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Oliver Elphick wrote:
-- Start of PGP signed section.
> On Wed, 2002-06-12 at 15:12, Tom Lane wrote:
> > Oliver Elphick <olly@lfix.co.uk> writes:
> > > The GUC option log_pid is not checked; LOG_PID is used by default,
> > > contrary to the documentation. This patch fixes it.
> >
> > No it doesn't (at least not unless you are also going to propose closing
> > and reopening syslog at SIGHUP to deal with changes in log_pid setting).
>
> ... partially fixes it :-(
>
> > I'm not convinced that there's anything wrong with the code anyway.
> > Do you expect log_timestamp to control whether timestamps are added to
> > syslog entries? If so you are out of luck. Perhaps we should just
>
> syslog() recognises LOG_PID as a flag to request logging of PIDs; there
> is no equivalent for the timestamp, of course.
>
> > tweak the documentation to make it clearer that log_pid and
> > log_timestamp only control the format of non-syslog logging.
>
> OK.
>
> If that is its real purpose, I think both I and the original submitter
> misunderstood the intention of this parameter, which certainly doesn't
> distinguish between logs via syslog and messages to the log file:
>
> LOG_PID (boolean)
>
> Prefixes each server log message with the process ID of the backend
> process. This is useful to sort out which messages pertain to which
> connection. The default is off.
>
>
> Perhaps that should read:
>
>
> LOG_PID (boolean)
>
> Prefixes each server message in the logfile with the process ID of
> the backend process. This is useful to sort out which messages
> pertain to which connection. The default is off. This parameter
> does not affect messages logged via syslog(), which always contain
> the process ID.
>
> I don't actually consider it a good thing to omit the PID in syslog, so
> I am happy that this change is not necessary, provided that the
> distinction between syslog and logfile is made clear.
>
> --
> Oliver Elphick Oliver.Elphick@lfix.co.uk
> Isle of Wight http://www.lfix.co.uk/oliver
> GPG: 1024D/3E1D0C1C: CA12 09E0 E8D5 8870 5839 932A 614D 4C34 3E1D 0C1C
>
> "And he said unto Jesus, Lord, remember me when thou
> comest into thy kingdom. And Jesus said unto him,
> Verily I say unto thee, To day shalt thou be with me
> in paradise." Luke 23:42,43
-- End of PGP section.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Patch applied. Thanks. --------------------------------------------------------------------------- Oliver Elphick wrote: Checking application/pgp-signature: FAILURE -- Start of PGP signed section. > On Wed, 2002-06-12 at 15:12, Tom Lane wrote: > > Oliver Elphick <olly@lfix.co.uk> writes: > > > The GUC option log_pid is not checked; LOG_PID is used by default, > > > contrary to the documentation. This patch fixes it. > > > > No it doesn't (at least not unless you are also going to propose closing > > and reopening syslog at SIGHUP to deal with changes in log_pid setting). > > ... partially fixes it :-( > > > I'm not convinced that there's anything wrong with the code anyway. > > Do you expect log_timestamp to control whether timestamps are added to > > syslog entries? If so you are out of luck. Perhaps we should just > > syslog() recognises LOG_PID as a flag to request logging of PIDs; there > is no equivalent for the timestamp, of course. > > > tweak the documentation to make it clearer that log_pid and > > log_timestamp only control the format of non-syslog logging. > > OK. > > If that is its real purpose, I think both I and the original submitter > misunderstood the intention of this parameter, which certainly doesn't > distinguish between logs via syslog and messages to the log file: > > LOG_PID (boolean) > > Prefixes each server log message with the process ID of the backend > process. This is useful to sort out which messages pertain to which > connection. The default is off. > > > Perhaps that should read: > > > LOG_PID (boolean) > > Prefixes each server message in the logfile with the process ID of > the backend process. This is useful to sort out which messages > pertain to which connection. The default is off. This parameter > does not affect messages logged via syslog(), which always contain > the process ID. > > I don't actually consider it a good thing to omit the PID in syslog, so > I am happy that this change is not necessary, provided that the > distinction between syslog and logfile is made clear. > > -- > Oliver Elphick Oliver.Elphick@lfix.co.uk > Isle of Wight http://www.lfix.co.uk/oliver > GPG: 1024D/3E1D0C1C: CA12 09E0 E8D5 8870 5839 932A 614D 4C34 3E1D 0C1C > > "And he said unto Jesus, Lord, remember me when thou > comest into thy kingdom. And Jesus said unto him, > Verily I say unto thee, To day shalt thou be with me > in paradise." Luke 23:42,43 -- End of PGP section, PGP failed! -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026