Incorporate strerror_r() into src/port/snprintf.c, too.

This provides the features that used to exist in useful_strerror()
for users of strerror_r(), too.  Also, standardize on the GNU convention
that strerror_r returns a char pointer that may not be NULL.

I notice that libpq's win32.c contains a variant version of strerror_r
that probably ought to be folded into strerror.c.  But lacking a
Windows environment, I should leave that to somebody else.

Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us

diff --git a/src/include/port.h b/src/include/port.h
index acf8d5fefd742c4642f5d155a0ebf95e9ad9a831..abbe1ad9a1a6db87bf44c4ff0f32501be384cc44 100644 (file)
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -193,6 +193,11 @@ extern int pg_printf(const char *fmt,...) pg_attribute_printf(1, 2);
 extern char *pg_strerror(int errnum);
 #define strerror pg_strerror
 
+/* Likewise for strerror_r(); note we prefer the GNU API for that */
+extern char *pg_strerror_r(int errnum, char *buf, size_t buflen);
+#define strerror_r pg_strerror_r
+#define PG_STRERROR_R_BUFLEN 256       /* Recommended buffer size for strerror_r */
+
 /* Portable prompt handling */
 extern void simple_prompt(const char *prompt, char *destination, size_t destlen,
                          bool echo);
@@ -428,8 +433,6 @@ extern char *dlerror(void);
 #endif
 
 /* thread.h */
-extern char *pqStrerror(int errnum, char *strerrbuf, size_t buflen);
-
 #ifndef WIN32
 extern int pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer,
                   size_t buflen, struct passwd **result);

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 540aba98b37119fb9f841f7c13d02e22e53abbfe..92641fe5e9acc37f6301fb5c89f06e49fe46ef71 100644 (file)
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -756,11 +756,11 @@ pg_local_sendauth(PGconn *conn)
 
        if (sendmsg(conn->sock, &msg, 0) == -1)
        {
-               char            sebuf[256];
+               char            sebuf[PG_STRERROR_R_BUFLEN];
 
                printfPQExpBuffer(&conn->errorMessage,
                                                  "pg_local_sendauth: sendmsg: %s\n",
-                                                 pqStrerror(errno, sebuf, sizeof(sebuf)));
+                                                 strerror_r(errno, sebuf, sizeof(sebuf)));
                return STATUS_ERROR;
        }
        return STATUS_OK;
@@ -1098,7 +1098,7 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
                        printfPQExpBuffer(errorMessage,
                                                          libpq_gettext("could not look up local user ID %d: %s\n"),
                                                          (int) user_id,
-                                                         pqStrerror(pwerr, pwdbuf, sizeof(pwdbuf)));
+                                                         strerror_r(pwerr, pwdbuf, sizeof(pwdbuf)));
                else
                        printfPQExpBuffer(errorMessage,
                                                          libpq_gettext("local user with ID %d does not exist\n"),

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 45bc067921c807aa1a9abb3895ee5e9795351324..d001bc513d30ba855bec068e9a2c598c12641ba7 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1459,7 +1459,7 @@ connectNoDelay(PGconn *conn)
                                   (char *) &on,
                                   sizeof(on)) < 0)
        {
-               char            sebuf[256];
+               char            sebuf[PG_STRERROR_R_BUFLEN];
 
                appendPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("could not set socket to TCP no delay mode: %s\n"),
@@ -1480,7 +1480,7 @@ connectNoDelay(PGconn *conn)
 static void
 connectFailureMessage(PGconn *conn, int errorno)
 {
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
 
 #ifdef HAVE_UNIX_SOCKETS
        if (IS_AF_UNIX(conn->raddr.addr.ss_family))
@@ -1637,7 +1637,7 @@ setKeepalivesIdle(PGconn *conn)
        if (setsockopt(conn->sock, IPPROTO_TCP, PG_TCP_KEEPALIVE_IDLE,
                                   (char *) &idle, sizeof(idle)) < 0)
        {
-               char            sebuf[256];
+               char            sebuf[PG_STRERROR_R_BUFLEN];
 
                appendPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("setsockopt(%s) failed: %s\n"),
@@ -1671,7 +1671,7 @@ setKeepalivesInterval(PGconn *conn)
        if (setsockopt(conn->sock, IPPROTO_TCP, TCP_KEEPINTVL,
                                   (char *) &interval, sizeof(interval)) < 0)
        {
-               char            sebuf[256];
+               char            sebuf[PG_STRERROR_R_BUFLEN];
 
                appendPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("setsockopt(%s) failed: %s\n"),
@@ -1706,7 +1706,7 @@ setKeepalivesCount(PGconn *conn)
        if (setsockopt(conn->sock, IPPROTO_TCP, TCP_KEEPCNT,
                                   (char *) &count, sizeof(count)) < 0)
        {
-               char            sebuf[256];
+               char            sebuf[PG_STRERROR_R_BUFLEN];
 
                appendPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("setsockopt(%s) failed: %s\n"),
@@ -2036,7 +2036,7 @@ PQconnectPoll(PGconn *conn)
        bool            reset_connection_state_machine = false;
        bool            need_new_connection = false;
        PGresult   *res;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
        int                     optval;
        PQExpBufferData savedMessage;
 
@@ -2580,7 +2580,7 @@ keep_going:                                               /* We will come back to here until there is
                                                else
                                                        appendPQExpBuffer(&conn->errorMessage,
                                                                                          libpq_gettext("could not get peer credentials: %s\n"),
-                                                                                         pqStrerror(errno, sebuf, sizeof(sebuf)));
+                                                                                         strerror_r(errno, sebuf, sizeof(sebuf)));
                                                goto error_return;
                                        }
 
@@ -2591,7 +2591,7 @@ keep_going:                                               /* We will come back to here until there is
                                                        appendPQExpBuffer(&conn->errorMessage,
                                                                                          libpq_gettext("could not look up local user ID %d: %s\n"),
                                                                                          (int) uid,
-                                                                                         pqStrerror(passerr, sebuf, sizeof(sebuf)));
+                                                                                         strerror_r(passerr, sebuf, sizeof(sebuf)));
                                                else
                                                        appendPQExpBuffer(&conn->errorMessage,
                                                                                          libpq_gettext("local user with ID %d does not exist\n"),
@@ -3953,7 +3953,7 @@ internal_cancel(SockAddr *raddr, int be_pid, int be_key,
 {
        int                     save_errno = SOCK_ERRNO;
        pgsocket        tmpsock = PGINVALID_SOCKET;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
        int                     maxlen;
        struct
        {

diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c
index a5ad3af2580bbfb61f6dec3a4edcf65911f79df9..b9caa229669abe825eff2d5b739355e815d92442 100644 (file)
--- a/src/interfaces/libpq/fe-lobj.c
+++ b/src/interfaces/libpq/fe-lobj.c
@@ -694,7 +694,7 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
        char            buf[LO_BUFSIZE];
        Oid                     lobjOid;
        int                     lobj;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
 
        /*
         * open the file to be read in
@@ -704,7 +704,7 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
        {                                                       /* error */
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("could not open file \"%s\": %s\n"),
-                                                 filename, pqStrerror(errno, sebuf, sizeof(sebuf)));
+                                                 filename, strerror_r(errno, sebuf, sizeof(sebuf)));
                return InvalidOid;
        }
 
@@ -760,7 +760,7 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("could not read from file \"%s\": %s\n"),
                                                  filename,
-                                                 pqStrerror(save_errno, sebuf, sizeof(sebuf)));
+                                                 strerror_r(save_errno, sebuf, sizeof(sebuf)));
                return InvalidOid;
        }
 
@@ -789,7 +789,7 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
                                tmp;
        char            buf[LO_BUFSIZE];
        int                     lobj;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
 
        /*
         * open the large object.
@@ -814,7 +814,7 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("could not open file \"%s\": %s\n"),
                                                  filename,
-                                                 pqStrerror(save_errno, sebuf, sizeof(sebuf)));
+                                                 strerror_r(save_errno, sebuf, sizeof(sebuf)));
                return -1;
        }
 
@@ -834,7 +834,7 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
                        printfPQExpBuffer(&conn->errorMessage,
                                                          libpq_gettext("could not write to file \"%s\": %s\n"),
                                                          filename,
-                                                         pqStrerror(save_errno, sebuf, sizeof(sebuf)));
+                                                         strerror_r(save_errno, sebuf, sizeof(sebuf)));
                        return -1;
                }
        }
@@ -857,7 +857,7 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
        {
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("could not write to file \"%s\": %s\n"),
-                                                 filename, pqStrerror(errno, sebuf, sizeof(sebuf)));
+                                                 filename, strerror_r(errno, sebuf, sizeof(sebuf)));
                result = -1;
        }
 

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 2a6637fdda398f0b3d4b1719322bf3fa3ef54672..46ece1a14c8d31692544491bb8dc1edc084d3c93 100644 (file)
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1071,7 +1071,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
 
        if (result < 0)
        {
-               char            sebuf[256];
+               char            sebuf[PG_STRERROR_R_BUFLEN];
 
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("select() failed: %s\n"),

diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index bbae8eff813906cc97b751677153ee6891e9e4bd..beca3492e8dcf1a2ba18f8ddaf2569d57bbaed8f 100644 (file)
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -142,7 +142,7 @@ pgtls_read(PGconn *conn, void *ptr, size_t len)
 {
        ssize_t         n;
        int                     result_errno = 0;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
        int                     err;
        unsigned long ecode;
 
@@ -272,7 +272,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
 {
        ssize_t         n;
        int                     result_errno = 0;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
        int                     err;
        unsigned long ecode;
 
@@ -443,7 +443,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 
        return cert_hash;
 }
-#endif /* HAVE_X509_GET_SIGNATURE_NID */
+#endif                                                 /* HAVE_X509_GET_SIGNATURE_NID */
 
 /* ------------------------------------------------------------ */
 /*                                             OpenSSL specific code                                   */
@@ -780,7 +780,7 @@ initialize_SSL(PGconn *conn)
        struct stat buf;
        char            homedir[MAXPGPATH];
        char            fnbuf[MAXPGPATH];
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
        bool            have_homedir;
        bool            have_cert;
        bool            have_rootcert;
@@ -941,7 +941,7 @@ initialize_SSL(PGconn *conn)
                {
                        printfPQExpBuffer(&conn->errorMessage,
                                                          libpq_gettext("could not open certificate file \"%s\": %s\n"),
-                                                         fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf)));
+                                                         fnbuf, strerror_r(errno, sebuf, sizeof(sebuf)));
                        SSL_CTX_free(SSL_context);
                        return -1;
                }
@@ -1212,7 +1212,7 @@ open_client_SSL(PGconn *conn)
 
                        case SSL_ERROR_SYSCALL:
                                {
-                                       char            sebuf[256];
+                                       char            sebuf[PG_STRERROR_R_BUFLEN];
 
                                        if (r == -1)
                                                printfPQExpBuffer(&conn->errorMessage,

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index f7dc249bf0c4cd4563c4ec8b1dc9a208c680271e..a06fc7dc824231215a256c275a619f77da08ed9f 100644 (file)
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -233,7 +233,7 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
 {
        ssize_t         n;
        int                     result_errno = 0;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
 
        n = recv(conn->sock, ptr, len, 0);
 
@@ -311,7 +311,7 @@ pqsecure_raw_write(PGconn *conn, const void *ptr, size_t len)
        ssize_t         n;
        int                     flags = 0;
        int                     result_errno = 0;
-       char            sebuf[256];
+       char            sebuf[PG_STRERROR_R_BUFLEN];
 
        DECLARE_SIGPIPE_INFO(spinfo);
 

diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index bdd8f9d9b29f64405a33218dac7baa90bde0c553..975ab33d025396ec77d19c7852b06a3986ea5eee 100644 (file)
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -773,7 +773,7 @@ extern char *libpq_ngettext(const char *msgid, const char *msgid_plural, unsigne
 #define SOCK_ERRNO_SET(e) WSASetLastError(e)
 #else
 #define SOCK_ERRNO errno
-#define SOCK_STRERROR pqStrerror
+#define SOCK_STRERROR strerror_r
 #define SOCK_ERRNO_SET(e) (errno = (e))
 #endif
 

diff --git a/src/port/strerror.c b/src/port/strerror.c
index ca6910d2e6c2e43488398ca9a56744721b8e0fe9..ba93815c50a2329716ea8a4802d4aada6af39f78 100644 (file)
--- a/src/port/strerror.c
+++ b/src/port/strerror.c
@@ -1,7 +1,7 @@
 /*-------------------------------------------------------------------------
  *
  * strerror.c
- *       Replacement for standard strerror() function
+ *       Replacements for standard strerror() and strerror_r() functions
  *
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -15,13 +15,16 @@
 #include "c.h"
 
 /*
- * Within this file, "strerror" means the platform's function not pg_strerror
+ * Within this file, "strerror" means the platform's function not pg_strerror,
+ * and likewise for "strerror_r"
  */
 #undef strerror
+#undef strerror_r
 
+static char *gnuish_strerror_r(int errnum, char *buf, size_t buflen);
 static char *get_errno_symbol(int errnum);
 #ifdef WIN32
-static char *win32_socket_strerror(int errnum);
+static char *win32_socket_strerror(int errnum, char *buf, size_t buflen);
 #endif
 
 
@@ -31,19 +34,28 @@ static char *win32_socket_strerror(int errnum);
 char *
 pg_strerror(int errnum)
 {
-       /* this buffer is only used if strerror() and get_errno_symbol() fail */
-       static char errorstr_buf[48];
+       static char errorstr_buf[PG_STRERROR_R_BUFLEN];
+
+       return pg_strerror_r(errnum, errorstr_buf, sizeof(errorstr_buf));
+}
+
+/*
+ * A slightly cleaned-up version of strerror_r()
+ */
+char *
+pg_strerror_r(int errnum, char *buf, size_t buflen)
+{
        char       *str;
 
        /* If it's a Windows Winsock error, that needs special handling */
 #ifdef WIN32
        /* Winsock error code range, per WinError.h */
        if (errnum >= 10000 && errnum <= 11999)
-               return win32_socket_strerror(errnum);
+               return win32_socket_strerror(errnum, buf, buflen);
 #endif
 
-       /* Try the platform's strerror() */
-       str = strerror(errnum);
+       /* Try the platform's strerror_r(), or maybe just strerror() */
+       str = gnuish_strerror_r(errnum, buf, buflen);
 
        /*
         * Some strerror()s return an empty string for out-of-range errno.  This
@@ -57,17 +69,42 @@ pg_strerror(int errnum)
 
        if (str == NULL)
        {
-               snprintf(errorstr_buf, sizeof(errorstr_buf),
-               /*------
-                 translator: This string will be truncated at 47
-                 characters expanded. */
-                                _("operating system error %d"), errnum);
-               str = errorstr_buf;
+               snprintf(buf, buflen, _("operating system error %d"), errnum);
+               str = buf;
        }
 
        return str;
 }
 
+/*
+ * Simple wrapper to emulate GNU strerror_r if what the platform provides is
+ * POSIX.  Also, if platform lacks strerror_r altogether, fall back to plain
+ * strerror; it might not be very thread-safe, but tough luck.
+ */
+static char *
+gnuish_strerror_r(int errnum, char *buf, size_t buflen)
+{
+#ifdef HAVE_STRERROR_R
+#ifdef STRERROR_R_INT
+       /* POSIX API */
+       if (strerror_r(errnum, buf, buflen) == 0)
+               return buf;
+       return NULL;                            /* let caller deal with failure */
+#else
+       /* GNU API */
+       return strerror_r(errnum, buf, buflen);
+#endif
+#else                                                  /* !HAVE_STRERROR_R */
+       char       *sbuf = strerror(errnum);
+
+       if (sbuf == NULL)                       /* can this still happen anywhere? */
+               return NULL;
+       /* To minimize thread-unsafety hazard, copy into caller's buffer */
+       strlcpy(buf, sbuf, buflen);
+       return buf;
+#endif
+}
+
 /*
  * Returns a symbol (e.g. "ENOENT") for an errno code.
  * Returns NULL if the code is unrecognized.
@@ -247,9 +284,8 @@ get_errno_symbol(int errnum)
  * Windows' strerror() doesn't know the Winsock codes, so handle them this way
  */
 static char *
-win32_socket_strerror(int errnum)
+win32_socket_strerror(int errnum, char *buf, size_t buflen)
 {
-       static char wserrbuf[256];
        static HANDLE handleDLL = INVALID_HANDLE_VALUE;
 
        if (handleDLL == INVALID_HANDLE_VALUE)
@@ -258,28 +294,29 @@ win32_socket_strerror(int errnum)
                                                                  DONT_RESOLVE_DLL_REFERENCES | LOAD_LIBRARY_AS_DATAFILE);
                if (handleDLL == NULL)
                {
-                       sprintf(wserrbuf, "winsock error %d (could not load netmsg.dll to translate: error code %lu)",
-                                       errnum, GetLastError());
-                       return wserrbuf;
+                       snprintf(buf, buflen,
+                                        "winsock error %d (could not load netmsg.dll to translate: error code %lu)",
+                                        errnum, GetLastError());
+                       return buf;
                }
        }
 
-       ZeroMemory(&wserrbuf, sizeof(wserrbuf));
+       ZeroMemory(buf, buflen);
        if (FormatMessage(FORMAT_MESSAGE_IGNORE_INSERTS |
                                          FORMAT_MESSAGE_FROM_SYSTEM |
                                          FORMAT_MESSAGE_FROM_HMODULE,
                                          handleDLL,
                                          errnum,
                                          MAKELANGID(LANG_ENGLISH, SUBLANG_DEFAULT),
-                                         wserrbuf,
-                                         sizeof(wserrbuf) - 1,
+                                         buf,
+                                         buflen - 1,
                                          NULL) == 0)
        {
                /* Failed to get id */
-               sprintf(wserrbuf, "unrecognized winsock error %d", errnum);
+               snprintf(buf, buflen, "unrecognized winsock error %d", errnum);
        }
 
-       return wserrbuf;
+       return buf;
 }
 
 #endif                                                 /* WIN32 */

diff --git a/src/port/thread.c b/src/port/thread.c
index da2df1f80876f62dc4a2e12bbcc446fe2ab5141f..8e0c7df73a896a7ec1063c90d9ff96ae422afc0b 100644 (file)
--- a/src/port/thread.c
+++ b/src/port/thread.c
@@ -53,33 +53,6 @@
  */
 
 
-/*
- * Wrapper around strerror and strerror_r to use the former if it is
- * available and also return a more useful value (the error string).
- */
-char *
-pqStrerror(int errnum, char *strerrbuf, size_t buflen)
-{
-#if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(HAVE_STRERROR_R)
-       /* reentrant strerror_r is available */
-#ifdef STRERROR_R_INT
-       /* SUSv3 version */
-       if (strerror_r(errnum, strerrbuf, buflen) == 0)
-               return strerrbuf;
-       else
-               return "Unknown error";
-#else
-       /* GNU libc */
-       return strerror_r(errnum, strerrbuf, buflen);
-#endif
-#else
-       /* no strerror_r() available, just use strerror */
-       strlcpy(strerrbuf, strerror(errnum), buflen);
-
-       return strerrbuf;
-#endif
-}
-
 /*
  * Wrapper around getpwuid() or getpwuid_r() to mimic POSIX getpwuid_r()
  * behaviour, if that function is not available or required.

diff --git a/src/test/thread/thread_test.c b/src/test/thread/thread_test.c
index 381607324d3c07bee15f5adc80f23cfac4ca1f05..31452317a6ec668c545e00f1aaff4e1c66cb009c 100644 (file)
--- a/src/test/thread/thread_test.c
+++ b/src/test/thread/thread_test.c
@@ -22,6 +22,9 @@
 
 #if !defined(IN_CONFIGURE) && !defined(WIN32)
 #include "postgres.h"
+
+/* we want to know what the native strerror does, not pg_strerror */
+#undef strerror
 #endif
 
 #include <stdio.h>
@@ -197,7 +200,7 @@ main(int argc, char *argv[])
        /* report results */
 
 #ifdef HAVE_STRERROR_R
-       printf("Your system has sterror_r();  it does not need strerror().\n");
+       printf("Your system has strerror_r(); it does not need strerror().\n");
 #else
        printf("Your system uses strerror() which is ");
        if (strerror_threadsafe)