Badly designed error reporting code in controldata_utils.c

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: Badly designed error reporting code in controldata_utils.c
Date: 2016-03-06 21:24:14
Message-ID: 32292.1457299454@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

My attention was drawn to the log_error() stuff in controldata_utils.c by
the fact that buildfarm member pademelon spit up on it. The reason for
that compile failure is that pademelon's dinosaur of a compiler doesn't
support __VA_ARGS__. I do not feel a need to get into a discussion about
whether we should move our portability goalposts for the convenience of
this commit, because there are other reasons why this is a crummy solution
for error reporting:

* It uses elog() not ereport() for what seems a not-particularly-internal
error, which among other things means that an entirely inappropriate
errcode() will be reported.

* It relies on strerror(errno), not %m, which may not work reliably even
in elog() and certainly won't in ereport() (because of order-of-evaluation
uncertainties).

* Translatability of the error message in the frontend context seems
a bit dubious; generally we let translators work with the whole string
to be printed, not just part of it.

* It's randomly unlike every single other place we've addressed the
same problem. Everywhere else in src/common does it like this:

#ifndef FRONTEND
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("out of memory")));
#else
fprintf(stderr, _("out of memory\n"));
exit(EXIT_FAILURE);
#endif

and I think that's what this needs to do too, especially in view of the
fact that there are only two places that would have to be fixed anyway.

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2016-03-06 21:26:34 Re: Badly designed error reporting code in controldata_utils.c
Previous Message Peter Geoghegan 2016-03-06 20:25:55 Re: The plan for FDW-based sharding