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)