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

* 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
                 errmsg("out of memory")));
        fprintf(stderr, _("out of memory\n"));

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

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to