From 77708750733737fb02941e44de75f4e0fc26353d Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 12 Oct 2022 09:03:52 +0200 Subject: [PATCH v2 1/2] libpq error message refactoring libpq now contains a mix of error message strings that end with newlines and don't end with newlines, due to some newer code paths with new ways of passing errors around. This leads to confusion and mistakes both during development and translation. This adds new functions libpq_append_error() and libpq_append_conn_error() that encapsulate common code paths for producing error message strings. Notably, these functions append the newline, so that the string appearing in the code does not end with a newline. This makes (almost) all error message strings in libpq uniform in this regard (and also consistent with how we handle it outside of libpq code). (There are a few exceptions that are difficult to fit into this scheme, but they are only a few.) Discussion: https://www.postgresql.org/message-id/flat/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com --- src/interfaces/libpq/fe-misc.c | 59 ++++++++++++++++++++++++++++++ src/interfaces/libpq/libpq-int.h | 3 ++ src/interfaces/libpq/nls.mk | 8 +++- src/interfaces/libpq/pqexpbuffer.c | 4 +- src/interfaces/libpq/pqexpbuffer.h | 11 ++++++ 5 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 795500c59354..9e3d38d33710 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -1278,3 +1278,62 @@ libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n) } #endif /* ENABLE_NLS */ + + +/* + * Append a formatted string to the given buffer, after translation. A + * newline is automatically appended; the format should not end with a + * newline. + */ +void +libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...) +{ + int save_errno = errno; + bool done; + va_list args; + + Assert(fmt[strlen(fmt) - 1] != '\n'); + + if (PQExpBufferBroken(errorMessage)) + return; /* already failed */ + + /* Loop in case we have to retry after enlarging the buffer. */ + do + { + errno = save_errno; + va_start(args, fmt); + done = appendPQExpBufferVA(errorMessage, libpq_gettext(fmt), args); + va_end(args); + } while (!done); + + appendPQExpBufferChar(errorMessage, '\n'); +} + +/* + * Append a formatted string to the error message buffer of the given + * connection, after translation. A newline is automatically appended; the + * format should not end with a newline. + */ +void +libpq_append_conn_error(PGconn *conn, const char *fmt, ...) +{ + int save_errno = errno; + bool done; + va_list args; + + Assert(fmt[strlen(fmt) - 1] != '\n'); + + if (PQExpBufferBroken(&conn->errorMessage)) + return; /* already failed */ + + /* Loop in case we have to retry after enlarging the buffer. */ + do + { + errno = save_errno; + va_start(args, fmt); + done = appendPQExpBufferVA(&conn->errorMessage, libpq_gettext(fmt), args); + va_end(args); + } while (!done); + + appendPQExpBufferChar(&conn->errorMessage, '\n'); +} diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index c75ed63a2c62..c24645b46965 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -887,6 +887,9 @@ extern char *libpq_ngettext(const char *msgid, const char *msgid_plural, unsigne */ #undef _ +extern void libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...) pg_attribute_printf(2, 3); +extern void libpq_append_conn_error(PGconn *conn, const char *fmt, ...) pg_attribute_printf(2, 3); + /* * These macros are needed to let error-handling code be portable between * Unix and Windows. (ugh) diff --git a/src/interfaces/libpq/nls.mk b/src/interfaces/libpq/nls.mk index 9256b426c1d4..4df544ecef56 100644 --- a/src/interfaces/libpq/nls.mk +++ b/src/interfaces/libpq/nls.mk @@ -1,5 +1,9 @@ # src/interfaces/libpq/nls.mk CATALOG_NAME = libpq GETTEXT_FILES = fe-auth.c fe-auth-scram.c fe-connect.c fe-exec.c fe-gssapi-common.c fe-lobj.c fe-misc.c fe-protocol3.c fe-secure.c fe-secure-common.c fe-secure-gssapi.c fe-secure-openssl.c win32.c ../../port/thread.c -GETTEXT_TRIGGERS = libpq_gettext pqInternalNotice:2 -GETTEXT_FLAGS = libpq_gettext:1:pass-c-format pqInternalNotice:2:c-format +GETTEXT_TRIGGERS = libpq_append_conn_error:2 \ + libpq_append_error:2 \ + libpq_gettext pqInternalNotice:2 +GETTEXT_FLAGS = libpq_append_conn_error:2:c-format \ + libpq_append_error:2:c-format \ + libpq_gettext:1:pass-c-format pqInternalNotice:2:c-format diff --git a/src/interfaces/libpq/pqexpbuffer.c b/src/interfaces/libpq/pqexpbuffer.c index eb51e6d08840..10b32efdfebc 100644 --- a/src/interfaces/libpq/pqexpbuffer.c +++ b/src/interfaces/libpq/pqexpbuffer.c @@ -40,8 +40,6 @@ static const char oom_buffer[1] = ""; /* Need a char * for unconstify() compatibility */ static const char *oom_buffer_ptr = oom_buffer; -static bool appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args) pg_attribute_printf(2, 0); - /* * markPQExpBufferBroken @@ -292,7 +290,7 @@ appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) * Caution: callers must be sure to preserve their entry-time errno * when looping, in case the fmt contains "%m". */ -static bool +bool appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args) { size_t avail; diff --git a/src/interfaces/libpq/pqexpbuffer.h b/src/interfaces/libpq/pqexpbuffer.h index efd652c80a33..8845783a9a7a 100644 --- a/src/interfaces/libpq/pqexpbuffer.h +++ b/src/interfaces/libpq/pqexpbuffer.h @@ -157,6 +157,17 @@ extern void printfPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute */ extern void appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute_printf(2, 3); +/*------------------------ + * appendPQExpBufferVA + * Shared guts of printfPQExpBuffer/appendPQExpBuffer. + * Attempt to format data and append it to str. Returns true if done + * (either successful or hard failure), false if need to retry. + * + * Caution: callers must be sure to preserve their entry-time errno + * when looping, in case the fmt contains "%m". + */ +extern bool appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args) pg_attribute_printf(2, 0); + /*------------------------ * appendPQExpBufferStr * Append the given string to a PQExpBuffer, allocating more space -- 2.37.3