Re: Check return value of pclose() correctly

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>, Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-11-01T20:41:15Z
Lists: pgsql-hackers
On 01.11.22 06:52, Tom Lane wrote:
> I think there are two issues here.  POSIX says
> 
>      Upon successful return, pclose() shall return the termination status
>      of the command language interpreter. Otherwise, pclose() shall return
>      -1 and set errno to indicate the error.
> 
> That is, first you need to make sure that pclose returned a valid
> child process status, and then you need to decode that status.
> It's not obvious to me that -1 is disjoint from the set of possible
> child statuses.  Do we need to add some logic that clears and then
> checks errno?

This return convention is also used by system() and is widely used.  So 
I don't think we need to be concerned about this.

In practice, int is 4 bytes and WEXITSTATUS() and WTERMSIG() are one 
byte each, so they are probably in the lower bytes, and wouldn't 
accidentally make up a -1.

> Also, we have a number of places --- at least FreeDesc() and
> ClosePipeStream() --- that consider pclose()'s return value to be
> perfectly equivalent to that of close() etc, because they'll
> return either one without telling the caller which is which.
> It seems like we have to fix that if we want to issue sane
> error reports.

I think this works.  FreeDesc() returns the pclose() exit status to 
ClosePipeStream(), which returns it directly.  No interpretation is done 
within these functions.




Commits

  1. Check return value of pclose() correctly