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

@@ -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);

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"),

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
 	{

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;
 	}
 

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"),

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;
 
@@ -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,

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);
 

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
 

src/port/strerror.c

 /*-------------------------------------------------------------------------
  *
  * 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)",
+			snprintf(buf, buflen,
+					 "winsock error %d (could not load netmsg.dll to translate: error code %lu)",
 					 errnum, GetLastError());
-			return wserrbuf;
+			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 */

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.

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)