Re: pgsql: Check dup2() results in syslogger
От | Stephen Frost |
---|---|
Тема | Re: pgsql: Check dup2() results in syslogger |
Дата | |
Msg-id | 20140127153213.GE31026@tamriel.snowman.net обсуждение исходный текст |
Ответ на | Re: pgsql: Check dup2() results in syslogger (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: pgsql: Check dup2() results in syslogger
Re: pgsql: Check dup2() results in syslogger |
Список | pgsql-committers |
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > Check dup2() results in syslogger > > Consistently check the dup2() call results throughout syslogger.c. > > It's pretty unlikely that they'll error out, but if they do, > > ereport(FATAL) instead of blissfully continuing on. > > Meh. Have you actually tested that an ereport(FATAL) is capable of doing > anything sane right there, with so much syslogger initialization left to > do, and no working stderr? Given that we were already doing so later on in the same function, it struck me as appropriate. I agree that the case is a bit interesting to consider. > 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. > > Spotted by the Coverity scanner. > > I fear this is mere Coverity-appeasement that has broken code that used > to work. Those dup2() calls look likely to only fail in a case of running out of file handles or similar, which struck me as a good reason to ereport(FATAL), similar to the setsid() failure which is checked for below. I could have just done (void) dup2() instead to make it clear that we were intentionally ignoring the results to please Coverity, and probably should have adjusted the close() calls that way. The last adjustment to this code was actually to avoid the dup2() calls causing failures *regardless* of our ignoring the result code due to a Windows' feature in CRT called Paramter Validation (per Heikki's commit message). Heikki, any thoughts on the right answer here? I admit that I didn't go to the effort of forcing the dup2() calls to fail to see what happens, though I did play around w/ killing off the syslogger forcibly and making sure it came back, which appeared to work fine. Thanks, Stephen
Вложения
В списке pgsql-committers по дате отправления: