Re: Check return value of pclose() correctly

On 01.11.22 21:30, Peter Eisentraut wrote:
>>> There are some places where the return value is apparently intentionally
>>> ignored, such as in error recovery paths, or psql ignoring a failure to
>>> launch the pager.  (The intention can usually be inferred by the kind of
>>> error checking attached to the corresponding popen() call.)  But
>>> there are a
>>> few places in psql that I'm suspicious about that I have marked, but
>>> need to
>>> think about further.
>>
>> Hmm.  I would leave these out, I think.  setQFout() relies on the
>> result of openQueryOutputFile().  And this could make commands like
>> \watch less reliable.
>
> I don't quite understand what you are saying here.  My point is that,
> for example, setQFout() thinks it's important to check the result of
> popen() and write an error message, but it doesn't check the result of
> pclose() at all.  I don't think that makes sense in practice.

I have looked this over again. In these cases, if the piped-to command
is faulty, you get a "broken pipe" error anyway, so the effect of not
checking the pclose() result is negligible. So I have removed the
"FIXME" markers without further action.

There is also the question whether we want to check the exit status of a
user-supplied command, such as in pgbench's \setshell. I have dialed
back my patch there, since I don't know what the current practice in
pgbench scripts is. If the command fails badly, pgbench will probably
complain anyway about invalid output.

More important is that something like pg_upgrade does check the exit
status when it calls pg_controldata etc. That's what this patch
accomplishes.