Re: patch: garbage error strings in libpq

Tom Lane wrote:
> jtv@xs4all.nl writes:
>> Another approach would have been to make libpq_gettext() preserve errno.
>
> That seems like a far easier, cleaner, and more robust fix than this.

Provided that either:

(a) the C standard has added a sequence point between the arguments in a
function call, which AFAIK wasn't there before, or the sequence point was
there all along (and the compiler implements it);
(b) the compiler is sufficiently naive;
(c) you get lucky with instruction scheduling on your particular
architecture.

This is why I called this approach was "tempting," but didn't go for it.
I felt it was better to really fix the instances I found first, then see
what patterns emerge and refactor.

Like maybe a wrapper for printfPQExpBuffer() that takes a PGconn *, an
untranslated format string, and varargs; this in turn can do the
libpq_gettext().  That would cover all uses of printfPQExpBuffer() in
libpq--except for one of the out-of-memory errors where no translation is
done, which may have been unintentional (and this bug is again duplicated
in the code).


> Moreover I don't believe that this approach works either, as the result
> of strerror() is not guaranteed still usable after another strerror call
> (ie, it can use a static buffer repeatedly), so you'd still have the
> problem if libpq_gettext invokes strerror.  I suppose that a really
> robust solution would involve libpq_gettext saving errno, restoring
> errno, and invoking strerror() again ...

Check again.  The calls to strerror() are routed through pqStrerror()
which copies the error message to the buffer, or in the case of GNU
strerror_r(), at least ensures it is in some reusable location.


Jeroen