Horton

Thread

Commits

  1. Remove a couple of strerror() calls

  1. thread-safety: strerror_r()

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-02T19:45:03Z

    There are only a few (not necessarily thread-safe) strerror() calls in 
the backend; most other potential users use %m in a format string.

In two cases, the reason for using strerror() was that we needed to 
print the error message twice, and so errno has to be reset for the 
second time.  And/or some of this code is from before snprintf() gained 
%m support.  This can easily be simplified now.

The other is a workaround for OpenSSL that we have already handled in an 
equivalent way in libpq.

(And there is one in postmaster.c, but that one is before forking.)

I think we can apply these patches now to check this off the list of 
not-thread-safe functions to check.

  2. Re: thread-safety: strerror_r()

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-09-02T19:56:40Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> I think we can apply these patches now to check this off the list of 
> not-thread-safe functions to check.

+1 for the first patch.  I'm less happy with

-	static char errbuf[36];
+	static char errbuf[128];

As a minor point, shouldn't this be

+	static char errbuf[PG_STRERROR_R_BUFLEN];

But the bigger issue is that the use of a static buffer makes
this not thread-safe, so having it use strerror_r to fill that
buffer is just putting lipstick on a pig.  If we really want
to make this thread-ready, we need to adopt the approach used
in libpq's fe-secure-openssl.c, where callers have to free the
buffer later.  Or maybe we could just palloc the result, and
trust that it's not in a long-lived context?

			regards, tom lane

  3. Re: thread-safety: strerror_r()

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-04T12:54:09Z

    On 02.09.24 21:56, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> I think we can apply these patches now to check this off the list of
>> not-thread-safe functions to check.
> 
> +1 for the first patch.  I'm less happy with
> 
> -	static char errbuf[36];
> +	static char errbuf[128];
> 
> As a minor point, shouldn't this be
> 
> +	static char errbuf[PG_STRERROR_R_BUFLEN];
> 
> But the bigger issue is that the use of a static buffer makes
> this not thread-safe, so having it use strerror_r to fill that
> buffer is just putting lipstick on a pig.  If we really want
> to make this thread-ready, we need to adopt the approach used
> in libpq's fe-secure-openssl.c, where callers have to free the
> buffer later.  Or maybe we could just palloc the result, and
> trust that it's not in a long-lived context?

Ok, I have committed the first patch.  We can think about the best 
solution for the second issue when we get to the business end of all 
this.  The current situation doesn't really prevent making any progress 
on that.