Re: Bogus error handling in pg_upgrade

On Sun, Dec 29, 2013 at 12:25:04AM -0500, Tom Lane wrote:
> A credulous person might suppose that this chunk of code is designed
> to abort if pg_resetxlog fails:
> 
>     prep_status("Setting next transaction ID for new cluster");
>     exec_prog(UTILITY_LOG_FILE, NULL, true,
>               "\"%s/pg_resetxlog\" -f -x %u \"%s\"",
>               new_cluster.bindir, old_cluster.controldata.chkpnt_nxtxid,
>               new_cluster.pgdata);
>     check_ok();
> 
> In point of fact, it does no such thing, but blithely continues
> (even though pg_resetxlog has corrupted things horribly before failing).

Well, exec_prog() does this:
   result = system(cmd);
   if (result != 0)

So, is pg_resetxlog returning a zero value?  I am guessing it is.

> check_ok() is particularly badly named, since it contains not one iota
> of error checking.  misleadingly_claim_ok() would be a better name.
> 
> If this isn't broken-by-design, I'd like an explanation why not.

It is probably because it came from initdb.c, but I always read check_ok
as report_ok.  Should I rename it?

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +