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

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com
This commit is contained in:
Peter Eisentraut 2022-11-15 11:50:04 +01:00
parent d627ce3b70
commit 0873b2d354
5 changed files with 79 additions and 5 deletions

View File

@ -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 translating it. 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 translating it. 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');
}

View File

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

View File

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

View File

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

View File

@ -157,6 +157,16 @@ extern void printfPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute
*/
extern void appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute_printf(2, 3);
/*------------------------
* appendPQExpBufferVA
* 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