Badly designed error reporting code in controldata_utils.c - Mailing list pgsql-hackers

From Tom Lane
Subject Badly designed error reporting code in controldata_utils.c
Date
Msg-id 32292.1457299454@sss.pgh.pa.us
Whole thread Raw
Responses Re: Badly designed error reporting code in controldata_utils.c
List 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("outof 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



pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: The plan for FDW-based sharding
Next
From: Joe Conway
Date:
Subject: Re: Badly designed error reporting code in controldata_utils.c