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.