Re: Obsolete coding in fork_process.c

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Obsolete coding in fork_process.c
Date: 2014-05-02 03:07:51
Message-ID: 11846.1399000071@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Noah Misch <noah(at)leadboat(dot)com> writes:
> On Thu, May 01, 2014 at 08:44:46PM -0400, Tom Lane wrote:
>> You're only considering one aspect of the problem. Yeah, you might not
>> get duplicated output unless system() prints something before exec(),
>> but we would also like to have causality: that is, whatever we sent to
>> stdout before calling system() should appear there before anything the
>> child process sends to stdout.

> Good point. I suppose a couple of fflush() calls have negligible cost next to
> a system() or popen(). Introduce pg_popen()/pg_system(), and adopt a rule
> that they are [almost] our only callers of raw popen()/system()?

Meh. I'm not usually in favor of adopting nonstandard notation, and
this doesn't seem like a place to start. In particular, if you don't
want to use fflush(NULL) in these proposed wrappers, then call sites
are still going to have an issue with needing to do manual fflushes;
pg_regress.c's spawn_process is an example:

/*
* Must flush I/O buffers before fork. Ideally we'd use fflush(NULL) here
* ... does anyone still care about systems where that doesn't work?
*/
fflush(stdout);
fflush(stderr);
if (logfile)
fflush(logfile);

pid = fork();

I think that removing the need for fflush(stdout) and fflush(stderr)
in this context would mostly result in people forgetting to fflush
other output files. I'd rather have the two lines of boilerplate
(and a comment about why we're refusing to depend on fflush(NULL))
than take that risk.

Now, if you were willing to put fflush(NULL) into the wrappers,
I could get on board with that ;-)

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2014-05-02 03:57:04 Allowing empty target list in SELECT (1b4f7f93b4693858cb983af3cd557f6097dab67b)
Previous Message Tom Lane 2014-05-02 02:51:31 Re: need xmllint on borka