From 758ce9b7794845f95473c569155d29fcf0e2751b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 26 Sep 2018 12:35:57 -0400 Subject: [PATCH] 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 --- src/include/port.h | 7 +- src/interfaces/libpq/fe-auth.c | 6 +- src/interfaces/libpq/fe-connect.c | 18 ++--- src/interfaces/libpq/fe-lobj.c | 14 ++-- src/interfaces/libpq/fe-misc.c | 2 +- src/interfaces/libpq/fe-secure-openssl.c | 12 ++-- src/interfaces/libpq/fe-secure.c | 4 +- src/interfaces/libpq/libpq-int.h | 2 +- src/port/strerror.c | 85 +++++++++++++++++------- src/port/thread.c | 27 -------- src/test/thread/thread_test.c | 5 +- 11 files changed, 99 insertions(+), 83 deletions(-) diff --git a/src/include/port.h b/src/include/port.h index acf8d5fefd..abbe1ad9a1 100644 --- 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 540aba98b3..92641fe5e9 100644 --- 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 45bc067921..d001bc513d 100644 --- 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 a5ad3af258..b9caa22966 100644 --- 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 2a6637fdda..46ece1a14c 100644 --- 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 bbae8eff81..beca3492e8 100644 --- 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 f7dc249bf0..a06fc7dc82 100644 --- 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 bdd8f9d9b2..975ab33d02 100644 --- 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 ca6910d2e6..ba93815c50 100644 --- 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 da2df1f808..8e0c7df73a 100644 --- 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 381607324d..31452317a6 100644 --- 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 @@ -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)