thread-safety: strerror_r()
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.
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
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.