Bruce Momjian <br...@momjian.us> writes:
> Can someone comment on the attached patch?  pg_upgrade was testing if
> system() returned a non-zero value, while I am thinking I should be
> adjusting system()'s return value with WEXITSTATUS().  

AFAIK it's not very good style to test the result as an integer, and
testing for -1 is not an improvement on that.  Actually it's a
disimprovement, because the only case where the standard guarantees
anything about the integer representation is that zero corresponds
to "exited with status 0".  See the Single Unix Spec, wherein system's
result code is defined in terms of wait's, and the wait man page saith

        If and only if the status returned is from a terminated child process
        that returned 0 from main() or passed 0 as the status argument to
        _exit() or exit(), the value stored at the location pointed to by
        stat_loc will be 0. Regardless of its value, this information may be
        interpreted using the following macros ...

If you want to do something different, then you could test for
WIFEXITED && WEXITSTATUS == 0.  (Testing the latter alone is flat
wrong.)  But I'm not particularly convinced that that's an improvement
on what's there now.  I note that your proposed patch would prevent
any possibility of printing debug information about failure cases,
since it loses the original result value.

In short: it's not broken now, but this patch would break it.

                        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to