On 01.11.22 06:35, Michael Paquier wrote:
- if (WIFEXITED(exitstatus))
+ if (exitstatus == -1)
+ {
+ snprintf(str, sizeof(str), "%m");
+ }
This addition in wait_result_to_str() looks inconsistent with the
existing callers of pclose() and ClosePipeStream() that check for -1
as exit status. copyfrom.c and basebackup_to_shell.c fall into this
category. Wouldn't it be better to unify everything?
With the above addition, the extra check for -1 at those existing places
could be removed.
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.
