Re: patch: garbage error strings in libpq

From: jtv(at)xs4all(dot)nl
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: jtv(at)xs4all(dot)nl, pgsql-patches(at)postgresql(dot)org
Subject: Re: patch: garbage error strings in libpq
Date: 2005-07-06 06:18:11
Message-ID: 16905.202.47.227.25.1120630691.squirrel@202.47.227.25
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Tom Lane wrote:
> jtv(at)xs4all(dot)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

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message jtv 2005-07-06 07:06:14 Re: Error handling fix in interfaces/libpq/fe-secure.c
Previous Message Bruce Momjian 2005-07-06 04:26:22 Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)