Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Please note also that the comment just above
>> this implies that we are deliberately ignoring any failures here, so I
>> think FATAL was probably the wrong thing in any case.
> We were explicitly ignoring the errors from the close(), I don't believe
> those comments were intended to apply to the dup2() calls as well, based
> on my reading of the commit history.
You shouldn't really raise that argument against the guy who made the
original commit in question ;-).
After re-reading a bit, the code here is indeed intentionally ignoring any
failure. The "if (redirection_done)" code block only gets executed in a
second or later incarnation of the syslogger. In the first (and usually
only) incarnation, syslogger inherits the postmaster's original stderr,
which we can hope points to somewhere worth bleating on, so we leave
stderr alone and make sure to write any syslogger-generated messages there
(see for instance the write_stderr call in write_syslogger_file).
However, if that incarnation dies and we have to spawn another, the second
and later incarnations will inherit a postmaster stderr that's already
redirected into the syslogger's input pipe. It is clearly a bad idea for
syslogger to try to write any bleats there, so we forcibly disconnect its
stderr in this case, acknowledging that that makes for a decrease in the
probability that we'll be able to report syslogger problems anywhere that
anybody could see them :-(.
Now ideally, the way we do that is to reconnect its stderr to /dev/null,
but if either the open(DEVNULL) or the dup2() calls were to fail, it will
presumably still work to leave the file descriptors just-plain-closed.
If the syslogger process then attempts to write on stderr, libc's internal
write() calls will fail, but so what? We wanted the output to go to the
bit bucket anyhow.
It's possible that under Windows "parameter validation", such a write
attempt would lead to a process abort. But I'm not sure how a forced
abort during startup is better than a possible future abort after a
failure that shouldn't happen in normal operation.
In short, this patch was ill considered. Please revert. If we need
to silence a Coverity complaint, perhaps a cast-to-void will do?
regards, tom lane
