pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: libpq error message refactoring
Date: Thu, 25 Aug 2022 16:34:26 +0200
Message-ID: <7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com> (raw)

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 has now gotten me confused a few too 
many times both during development and translation.  So I looked into 
whether we can unify this, similar to how we have done elsewhere (e.g., 
pg_upgrade).  I came up with the attached patch.  It's not complete, but 
it shows the idea and it looks like a nice simplification to me. 
Thoughts on this approach?

Attachments:

v1-0001-WIP-libpq_append_error.patch (text/plain, 40.4K) download From e547d9993232e20c9e696bd7facbcb3de4afdd6c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter@eisentraut.org> Date: Thu, 4 Aug 2022 23:31:58 +0300 Subject: [PATCH v1] WIP: libpq_append_error --- src/interfaces/libpq/fe-connect.c | 271 +++++++++++------------------ src/interfaces/libpq/fe-exec.c | 109 ++++-------- src/interfaces/libpq/fe-misc.c | 15 ++ src/interfaces/libpq/libpq-int.h | 2 + src/interfaces/libpq/nls.mk | 4 +- src/interfaces/libpq/pqexpbuffer.c | 27 ++- src/interfaces/libpq/pqexpbuffer.h | 2 + 7 files changed, 186 insertions(+), 244 deletions(-) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 8cefef20d1..79b9392858 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -896,8 +896,7 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions) *connmember = strdup(tmp); if (*connmember == NULL) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); return false; } } @@ -1079,9 +1078,8 @@ connectOptions2(PGconn *conn) if (more || i != conn->nconnhost) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not match %d host names to %d hostaddr values\n"), - count_comma_separated_elems(conn->pghost), conn->nconnhost); + libpq_append_error(conn, "could not match %d host names to %d hostaddr values", + count_comma_separated_elems(conn->pghost), conn->nconnhost); return false; } } @@ -1160,9 +1158,8 @@ connectOptions2(PGconn *conn) else if (more || i != conn->nconnhost) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not match %d port numbers to %d hosts\n"), - count_comma_separated_elems(conn->pgport), conn->nconnhost); + libpq_append_error(conn, "could not match %d port numbers to %d hosts", + count_comma_separated_elems(conn->pgport), conn->nconnhost); return false; } } @@ -1250,9 +1247,8 @@ connectOptions2(PGconn *conn) && strcmp(conn->channel_binding, "require") != 0) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid %s value: \"%s\"\n"), - "channel_binding", conn->channel_binding); + libpq_append_error(conn, "invalid %s value: \"%s\"", + "channel_binding", conn->channel_binding); return false; } } @@ -1276,9 +1272,8 @@ connectOptions2(PGconn *conn) && strcmp(conn->sslmode, "verify-full") != 0) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid %s value: \"%s\"\n"), - "sslmode", conn->sslmode); + libpq_append_error(conn, "invalid %s value: \"%s\"", + "sslmode", conn->sslmode); return false; } @@ -1297,9 +1292,8 @@ connectOptions2(PGconn *conn) case 'r': /* "require" */ case 'v': /* "verify-ca" or "verify-full" */ conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("sslmode value \"%s\" invalid when SSL support is not compiled in\n"), - conn->sslmode); + libpq_append_error(conn, "sslmode value \"%s\" invalid when SSL support is not compiled in", + conn->sslmode); return false; } #endif @@ -1318,19 +1312,17 @@ connectOptions2(PGconn *conn) if (!sslVerifyProtocolVersion(conn->ssl_min_protocol_version)) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid %s value: \"%s\"\n"), - "ssl_min_protocol_version", - conn->ssl_min_protocol_version); + libpq_append_error(conn, "invalid %s value: \"%s\"", + "ssl_min_protocol_version", + conn->ssl_min_protocol_version); return false; } if (!sslVerifyProtocolVersion(conn->ssl_max_protocol_version)) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid %s value: \"%s\"\n"), - "ssl_max_protocol_version", - conn->ssl_max_protocol_version); + libpq_append_error(conn, "invalid %s value: \"%s\"", + "ssl_max_protocol_version", + conn->ssl_max_protocol_version); return false; } @@ -1345,8 +1337,7 @@ connectOptions2(PGconn *conn) conn->ssl_max_protocol_version)) { conn->status = CONNECTION_BAD; - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("invalid SSL protocol version range\n")); + libpq_append_error(conn, "invalid SSL protocol version range"); return false; } @@ -1360,19 +1351,15 @@ connectOptions2(PGconn *conn) strcmp(conn->gssencmode, "require") != 0) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid %s value: \"%s\"\n"), - "gssencmode", - conn->gssencmode); + libpq_append_error(conn, "invalid %s value: \"%s\"", "gssencmode", conn->gssencmode); return false; } #ifndef ENABLE_GSS if (strcmp(conn->gssencmode, "require") == 0) { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("gssencmode value \"%s\" invalid when GSSAPI support is not compiled in\n"), - conn->gssencmode); + libpq_append_error(conn, "gssencmode value \"%s\" invalid when GSSAPI support is not compiled in", + conn->gssencmode); return false; } #endif @@ -1404,10 +1391,9 @@ connectOptions2(PGconn *conn) else { conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid %s value: \"%s\"\n"), - "target_session_attrs", - conn->target_session_attrs); + libpq_append_error(conn, "invalid %s value: \"%s\"", + "target_session_attrs", + conn->target_session_attrs); return false; } } @@ -1437,8 +1423,7 @@ connectOptions2(PGconn *conn) oom_error: conn->status = CONNECTION_BAD; - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); return false; } @@ -1600,8 +1585,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions, oom_error: conn->status = CONNECTION_BAD; - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); return conn; } @@ -1624,9 +1608,8 @@ connectNoDelay(PGconn *conn) { char sebuf[PG_STRERROR_R_BUFLEN]; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not set socket to TCP no delay mode: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not set socket to TCP no delay mode: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); return 0; } #endif @@ -1740,11 +1723,9 @@ connectFailureMessage(PGconn *conn, int errorno) SOCK_STRERROR(errorno, sebuf, sizeof(sebuf))); if (conn->raddr.addr.ss_family == AF_UNIX) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("\tIs the server running locally and accepting connections on that socket?\n")); + libpq_append_error(conn, "\tIs the server running locally and accepting connections on that socket?"); else - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("\tIs the server running on that host and accepting TCP/IP connections?\n")); + libpq_append_error(conn, "\tIs the server running on that host and accepting TCP/IP connections?"); } /* @@ -1807,9 +1788,8 @@ parse_int_param(const char *value, int *result, PGconn *conn, return true; error: - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid integer value \"%s\" for connection option \"%s\"\n"), - value, context); + libpq_append_error(conn, "invalid integer value \"%s\" for connection option \"%s\"", + value, context); return false; } @@ -1837,11 +1817,10 @@ setKeepalivesIdle(PGconn *conn) { char sebuf[PG_STRERROR_R_BUFLEN]; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("%s(%s) failed: %s\n"), - "setsockopt", - PG_TCP_KEEPALIVE_IDLE_STR, - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "%s(%s) failed: %s", + "setsockopt", + PG_TCP_KEEPALIVE_IDLE_STR, + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); return 0; } #endif @@ -1872,11 +1851,10 @@ setKeepalivesInterval(PGconn *conn) { char sebuf[PG_STRERROR_R_BUFLEN]; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("%s(%s) failed: %s\n"), - "setsockopt", - "TCP_KEEPINTVL", - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "%s(%s) failed: %s", + "setsockopt", + "TCP_KEEPINTVL", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); return 0; } #endif @@ -1908,11 +1886,10 @@ setKeepalivesCount(PGconn *conn) { char sebuf[PG_STRERROR_R_BUFLEN]; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("%s(%s) failed: %s\n"), - "setsockopt", - "TCP_KEEPCNT", - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "%s(%s) failed: %s", + "setsockopt", + "TCP_KEEPCNT", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); return 0; } #endif @@ -1973,8 +1950,7 @@ prepKeepalivesWin32(PGconn *conn) if (!setKeepalivesWin32(conn->sock, idle, interval)) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("%s(%s) failed: error code %d\n"), + libpq_append_error(conn, "%s(%s) failed: error code %d", "WSAIoctl", "SIO_KEEPALIVE_VALS", WSAGetLastError()); return 0; @@ -2008,11 +1984,10 @@ setTCPUserTimeout(PGconn *conn) { char sebuf[256]; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("%s(%s) failed: %s\n"), - "setsockopt", - "TCP_USER_TIMEOUT", - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "%s(%s) failed: %s", + "setsockopt", + "TCP_USER_TIMEOUT", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); return 0; } #endif @@ -2288,8 +2263,7 @@ PQconnectPoll(PGconn *conn) break; default: - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("invalid connection state, probably indicative of memory corruption\n")); + libpq_append_error(conn, "invalid connection state, probably indicative of memory corruption"); goto error_return; } @@ -2367,9 +2341,7 @@ PQconnectPoll(PGconn *conn) if (thisport < 1 || thisport > 65535) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid port number: \"%s\"\n"), - ch->port); + libpq_append_error(conn, "invalid port number: \"%s\"", ch->port); goto keep_going; } } @@ -2383,9 +2355,8 @@ PQconnectPoll(PGconn *conn) &conn->addrlist); if (ret || !conn->addrlist) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not translate host name \"%s\" to address: %s\n"), - ch->host, gai_strerror(ret)); + libpq_append_error(conn, "could not translate host name \"%s\" to address: %s", + ch->host, gai_strerror(ret)); goto keep_going; } break; @@ -2396,9 +2367,8 @@ PQconnectPoll(PGconn *conn) &conn->addrlist); if (ret || !conn->addrlist) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not parse network address \"%s\": %s\n"), - ch->hostaddr, gai_strerror(ret)); + libpq_append_error(conn, "could not parse network address \"%s\": %s", + ch->hostaddr, gai_strerror(ret)); goto keep_going; } break; @@ -2408,10 +2378,9 @@ PQconnectPoll(PGconn *conn) UNIXSOCK_PATH(portstr, thisport, ch->host); if (strlen(portstr) >= UNIXSOCK_PATH_BUFLEN) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("Unix-domain socket path \"%s\" is too long (maximum %d bytes)\n"), - portstr, - (int) (UNIXSOCK_PATH_BUFLEN - 1)); + libpq_append_error(conn, "Unix-domain socket path \"%s\" is too long (maximum %d bytes)", + portstr, + (int) (UNIXSOCK_PATH_BUFLEN - 1)); goto keep_going; } @@ -2423,9 +2392,8 @@ PQconnectPoll(PGconn *conn) &conn->addrlist); if (ret || !conn->addrlist) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not translate Unix-domain socket path \"%s\" to address: %s\n"), - portstr, gai_strerror(ret)); + libpq_append_error(conn, "could not translate Unix-domain socket path \"%s\" to address: %s", + portstr, gai_strerror(ret)); goto keep_going; } break; @@ -2546,9 +2514,8 @@ PQconnectPoll(PGconn *conn) goto keep_going; } emitHostIdentityInfo(conn, host_addr); - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not create socket: %s\n"), - SOCK_STRERROR(errorno, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not create socket: %s", + SOCK_STRERROR(errorno, sebuf, sizeof(sebuf))); goto error_return; } @@ -2577,9 +2544,8 @@ PQconnectPoll(PGconn *conn) } if (!pg_set_noblock(conn->sock)) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not set socket to nonblocking mode: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not set socket to nonblocking mode: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); conn->try_next_addr = true; goto keep_going; } @@ -2587,9 +2553,8 @@ PQconnectPoll(PGconn *conn) #ifdef F_SETFD if (fcntl(conn->sock, F_SETFD, FD_CLOEXEC) == -1) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not set socket to close-on-exec mode: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not set socket to close-on-exec mode: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); conn->try_next_addr = true; goto keep_going; } @@ -2605,8 +2570,7 @@ PQconnectPoll(PGconn *conn) if (usekeepalives < 0) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("keepalives parameter must be an integer\n")); + libpq_append_error(conn, "keepalives parameter must be an integer"); err = 1; } else if (usekeepalives == 0) @@ -2618,11 +2582,10 @@ PQconnectPoll(PGconn *conn) SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on)) < 0) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("%s(%s) failed: %s\n"), - "setsockopt", - "SO_KEEPALIVE", - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "%s(%s) failed: %s", + "setsockopt", + "SO_KEEPALIVE", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); err = 1; } else if (!setKeepalivesIdle(conn) @@ -2746,9 +2709,8 @@ PQconnectPoll(PGconn *conn) if (getsockopt(conn->sock, SOL_SOCKET, SO_ERROR, (char *) &optval, &optlen) == -1) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not get socket error status: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not get socket error status: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); goto error_return; } else if (optval != 0) @@ -2774,9 +2736,8 @@ PQconnectPoll(PGconn *conn) (struct sockaddr *) &conn->laddr.addr, &conn->laddr.salen) < 0) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not get client address from socket: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not get client address from socket: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); goto error_return; } @@ -2813,12 +2774,10 @@ PQconnectPoll(PGconn *conn) * stub */ if (errno == ENOSYS) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("requirepeer parameter is not supported on this platform\n")); + libpq_append_error(conn, "requirepeer parameter is not supported on this platform"); else - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not get peer credentials: %s\n"), - strerror_r(errno, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not get peer credentials: %s", + strerror_r(errno, sebuf, sizeof(sebuf))); goto error_return; } @@ -2830,9 +2789,8 @@ PQconnectPoll(PGconn *conn) if (strcmp(remote_username, conn->requirepeer) != 0) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("requirepeer specifies \"%s\", but actual peer user name is \"%s\"\n"), - conn->requirepeer, remote_username); + libpq_append_error(conn, "requirepeer specifies \"%s\", but actual peer user name is \"%s\"", + conn->requirepeer, remote_username); free(remote_username); goto error_return; } @@ -2872,9 +2830,8 @@ PQconnectPoll(PGconn *conn) if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not send GSSAPI negotiation packet: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not send GSSAPI negotiation packet: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); goto error_return; } @@ -2884,8 +2841,8 @@ PQconnectPoll(PGconn *conn) } else if (!conn->gctx && conn->gssencmode[0] == 'r') { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("GSSAPI encryption required but was impossible (possibly no credential cache, no server support, or using a local socket)\n")); + libpq_append_error(conn, + "GSSAPI encryption required but was impossible (possibly no credential cache, no server support, or using a local socket)"); goto error_return; } #endif @@ -2926,9 +2883,8 @@ PQconnectPoll(PGconn *conn) pv = pg_hton32(NEGOTIATE_SSL_CODE); if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not send SSL negotiation packet: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not send SSL negotiation packet: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); goto error_return; } /* Ok, wait for response */ @@ -2944,8 +2900,7 @@ PQconnectPoll(PGconn *conn) EnvironmentOptions); if (!startpacket) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); goto error_return; } @@ -2957,9 +2912,8 @@ PQconnectPoll(PGconn *conn) */ if (pqPacketSend(conn, 0, startpacket, packetlen) != STATUS_OK) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not send startup packet: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + libpq_append_error(conn, "could not send startup packet: %s", + SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); free(startpacket); goto error_return; } @@ -3033,8 +2987,7 @@ PQconnectPoll(PGconn *conn) * "verify-full" */ { /* Require SSL, but server does not want it */ - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("server does not support SSL, but SSL was required\n")); + libpq_append_error(conn, "server does not support SSL, but SSL was required"); goto error_return; } /* Otherwise, proceed with normal startup */ @@ -3060,9 +3013,8 @@ PQconnectPoll(PGconn *conn) } else { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("received invalid response to SSL negotiation: %c\n"), - SSLok); + libpq_append_error(conn, "received invalid response to SSL negotiation: %c", + SSLok); goto error_return; } } @@ -3081,8 +3033,7 @@ PQconnectPoll(PGconn *conn) */ if (conn->inCursor != conn->inEnd) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("received unencrypted data after SSL response\n")); + libpq_append_error(conn, "received unencrypted data after SSL response"); goto error_return; } @@ -3162,8 +3113,7 @@ PQconnectPoll(PGconn *conn) /* Server doesn't want GSSAPI; fall back if we can */ if (conn->gssencmode[0] == 'r') { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("server doesn't support GSSAPI encryption, but it was required\n")); + libpq_append_error(conn, "server doesn't support GSSAPI encryption, but it was required"); goto error_return; } @@ -3174,9 +3124,8 @@ PQconnectPoll(PGconn *conn) } else if (gss_ok != 'G') { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("received invalid response to GSSAPI negotiation: %c\n"), - gss_ok); + libpq_append_error(conn, "received invalid response to GSSAPI negotiation: %c", + gss_ok); goto error_return; } } @@ -3193,8 +3142,7 @@ PQconnectPoll(PGconn *conn) */ if (conn->inCursor != conn->inEnd) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("received unencrypted data after GSSAPI encryption response\n")); + libpq_append_error(conn, "received unencrypted data after GSSAPI encryption response"); goto error_return; } @@ -3253,9 +3201,8 @@ PQconnectPoll(PGconn *conn) */ if (!(beresp == 'R' || beresp == 'E')) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("expected authentication request from server, but received %c\n"), - beresp); + libpq_append_error(conn, "expected authentication request from server, but received %c", + beresp); goto error_return; } @@ -3278,9 +3225,8 @@ PQconnectPoll(PGconn *conn) */ if (beresp == 'R' && (msgLength < 8 || msgLength > 2000)) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("expected authentication request from server, but received %c\n"), - beresp); + libpq_append_error(conn, "expected authentication request from server, but received %c", + beresp); goto error_return; } @@ -3485,8 +3431,7 @@ PQconnectPoll(PGconn *conn) if (res) { if (res->resultStatus != PGRES_FATAL_ERROR) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("unexpected message from server during startup\n")); + libpq_append_error(conn, "unexpected message from server during startup"); else if (conn->send_appname && (conn->appname || conn->fbappname)) { @@ -3577,11 +3522,9 @@ PQconnectPoll(PGconn *conn) { /* Wrong server state, reject and try the next host */ if (conn->target_server_type == SERVER_TYPE_READ_WRITE) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("session is read-only\n")); + libpq_append_error(conn, "session is read-only"); else - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("session is not read-only\n")); + libpq_append_error(conn, "session is not read-only"); /* Close connection politely. */ conn->status = CONNECTION_OK; @@ -3634,11 +3577,9 @@ PQconnectPoll(PGconn *conn) { /* Wrong server state, reject and try the next host */ if (conn->target_server_type == SERVER_TYPE_PRIMARY) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("server is in hot standby mode\n")); + libpq_append_error(conn, "server is in hot standby mode"); else - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("server is not in hot standby mode\n")); + libpq_append_error(conn, "server is not in hot standby mode"); /* Close connection politely. */ conn->status = CONNECTION_OK; diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index bb874f7f50..28bee2cbed 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -1316,8 +1316,7 @@ pqAllocCmdQueueEntry(PGconn *conn) entry = (PGcmdQueueEntry *) malloc(sizeof(PGcmdQueueEntry)); if (entry == NULL) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); return NULL; } } @@ -1441,8 +1440,7 @@ PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery) /* check the argument */ if (!query) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("command string is a null pointer\n")); + libpq_append_error(conn, "command string is a null pointer"); return 0; } @@ -1567,15 +1565,13 @@ PQsendQueryParams(PGconn *conn, /* check the arguments */ if (!command) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("command string is a null pointer\n")); + libpq_append_error(conn, "command string is a null pointer"); return 0; } if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("number of parameters must be between 0 and %d\n"), - PQ_QUERY_PARAM_MAX_LIMIT); + libpq_append_error(conn, "number of parameters must be between 0 and %d", + PQ_QUERY_PARAM_MAX_LIMIT); return 0; } @@ -1610,21 +1606,18 @@ PQsendPrepare(PGconn *conn, /* check the arguments */ if (!stmtName) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("statement name is a null pointer\n")); + libpq_append_error(conn, "statement name is a null pointer"); return 0; } if (!query) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("command string is a null pointer\n")); + libpq_append_error(conn, "command string is a null pointer"); return 0; } if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("number of parameters must be between 0 and %d\n"), - PQ_QUERY_PARAM_MAX_LIMIT); + libpq_append_error(conn, "number of parameters must be between 0 and %d", + PQ_QUERY_PARAM_MAX_LIMIT); return 0; } @@ -1712,15 +1705,13 @@ PQsendQueryPrepared(PGconn *conn, /* check the arguments */ if (!stmtName) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("statement name is a null pointer\n")); + libpq_append_error(conn, "statement name is a null pointer"); return 0; } if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT) { - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("number of parameters must be between 0 and %d\n"), - PQ_QUERY_PARAM_MAX_LIMIT); + libpq_append_error(conn, "number of parameters must be between 0 and %d", + PQ_QUERY_PARAM_MAX_LIMIT); return 0; } @@ -1756,8 +1747,7 @@ PQsendQueryStart(PGconn *conn, bool newQuery) /* Don't try to send if we know there's no live connection. */ if (conn->status != CONNECTION_OK) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("no connection to the server\n")); + libpq_append_error(conn, "no connection to the server"); return false; } @@ -1765,8 +1755,7 @@ PQsendQueryStart(PGconn *conn, bool newQuery) if (conn->asyncStatus != PGASYNC_IDLE && conn->pipelineStatus == PQ_PIPELINE_OFF) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("another command is already in progress\n")); + libpq_append_error(conn, "another command is already in progress"); return false; } @@ -1796,8 +1785,7 @@ PQsendQueryStart(PGconn *conn, bool newQuery) case PGASYNC_COPY_IN: case PGASYNC_COPY_OUT: case PGASYNC_COPY_BOTH: - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("cannot queue commands during COPY\n")); + libpq_append_error(conn, "cannot queue commands during COPY"); return false; } } @@ -1914,8 +1902,7 @@ PQsendQueryGuts(PGconn *conn, nbytes = paramLengths[i]; else { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("length must be given for binary parameter\n")); + libpq_append_error(conn, "length must be given for binary parameter"); goto sendFailed; } } @@ -2237,9 +2224,7 @@ PQgetResult(PGconn *conn) res = getCopyResult(conn, PGRES_COPY_BOTH); break; default: - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("unexpected asyncStatus: %d\n"), - (int) conn->asyncStatus); + libpq_append_error(conn, "unexpected asyncStatus: %d", (int) conn->asyncStatus); pqSaveErrorResult(conn); conn->asyncStatus = PGASYNC_IDLE; /* try to restore valid state */ res = pqPrepareAsyncResult(conn); @@ -2411,8 +2396,7 @@ PQexecStart(PGconn *conn) if (conn->pipelineStatus != PQ_PIPELINE_OFF) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("synchronous command execution functions are not allowed in pipeline mode\n")); + libpq_append_error(conn, "synchronous command execution functions are not allowed in pipeline mode"); return false; } @@ -2445,8 +2429,7 @@ PQexecStart(PGconn *conn) else if (resultStatus == PGRES_COPY_BOTH) { /* We don't allow PQexec during COPY BOTH */ - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("PQexec not allowed during COPY BOTH\n")); + libpq_append_error(conn, "PQexec not allowed during COPY BOTH"); return false; } /* check for loss of connection, too */ @@ -2672,8 +2655,7 @@ PQputCopyData(PGconn *conn, const char *buffer, int nbytes) if (conn->asyncStatus != PGASYNC_COPY_IN && conn->asyncStatus != PGASYNC_COPY_BOTH) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("no COPY in progress\n")); + libpq_append_error(conn, "no COPY in progress"); return -1; } @@ -2728,8 +2710,7 @@ PQputCopyEnd(PGconn *conn, const char *errormsg) if (conn->asyncStatus != PGASYNC_COPY_IN && conn->asyncStatus != PGASYNC_COPY_BOTH) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("no COPY in progress\n")); + libpq_append_error(conn, "no COPY in progress"); return -1; } @@ -2797,8 +2778,7 @@ PQgetCopyData(PGconn *conn, char **buffer, int async) if (conn->asyncStatus != PGASYNC_COPY_OUT && conn->asyncStatus != PGASYNC_COPY_BOTH) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("no COPY in progress\n")); + libpq_append_error(conn, "no COPY in progress"); return -2; } return pqGetCopyData3(conn, buffer, async); @@ -2977,16 +2957,14 @@ PQfn(PGconn *conn, if (conn->pipelineStatus != PQ_PIPELINE_OFF) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("PQfn not allowed in pipeline mode\n")); + libpq_append_error(conn, "PQfn not allowed in pipeline mode"); return NULL; } if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE || pgHavePendingResult(conn)) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("connection in wrong state\n")); + libpq_append_error(conn, "connection in wrong state"); return NULL; } @@ -3029,8 +3007,7 @@ PQenterPipelineMode(PGconn *conn) if (conn->asyncStatus != PGASYNC_IDLE) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("cannot enter pipeline mode, connection not idle\n")); + libpq_append_error(conn, "cannot enter pipeline mode, connection not idle"); return 0; } @@ -3066,13 +3043,11 @@ PQexitPipelineMode(PGconn *conn) case PGASYNC_READY: case PGASYNC_READY_MORE: /* there are some uncollected results */ - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("cannot exit pipeline mode with uncollected results\n")); + libpq_append_error(conn, "cannot exit pipeline mode with uncollected results"); return 0; case PGASYNC_BUSY: - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("cannot exit pipeline mode while busy\n")); + libpq_append_error(conn, "cannot exit pipeline mode while busy"); return 0; case PGASYNC_IDLE: @@ -3083,15 +3058,13 @@ PQexitPipelineMode(PGconn *conn) case PGASYNC_COPY_IN: case PGASYNC_COPY_OUT: case PGASYNC_COPY_BOTH: - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("cannot exit pipeline mode while in COPY\n")); + libpq_append_error(conn, "cannot exit pipeline mode while in COPY"); } /* still work to process */ if (conn->cmd_queue_head != NULL) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("cannot exit pipeline mode with uncollected results\n")); + libpq_append_error(conn, "cannot exit pipeline mode with uncollected results"); return 0; } @@ -3206,8 +3179,7 @@ pqPipelineProcessQueue(PGconn *conn) conn->result = PQmakeEmptyPGresult(conn, PGRES_PIPELINE_ABORTED); if (!conn->result) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); pqSaveErrorResult(conn); return; } @@ -3250,8 +3222,7 @@ PQpipelineSync(PGconn *conn) if (conn->pipelineStatus == PQ_PIPELINE_OFF) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("cannot send pipeline when not in pipeline mode\n")); + libpq_append_error(conn, "cannot send pipeline when not in pipeline mode"); return 0; } @@ -3317,8 +3288,7 @@ PQsendFlushRequest(PGconn *conn) /* Don't try to send if we know there's no live connection. */ if (conn->status != CONNECTION_OK) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("no connection to the server\n")); + libpq_append_error(conn, "no connection to the server"); return 0; } @@ -3326,8 +3296,7 @@ PQsendFlushRequest(PGconn *conn) if (conn->asyncStatus != PGASYNC_IDLE && conn->pipelineStatus == PQ_PIPELINE_OFF) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("another command is already in progress\n")); + libpq_append_error(conn, "another command is already in progress"); return 0; } @@ -4063,8 +4032,7 @@ PQescapeStringInternal(PGconn *conn, if (error) *error = 1; if (conn) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("incomplete multibyte character\n")); + libpq_append_error(conn, "incomplete multibyte character"); for (; i < len; i++) { if (((size_t) (target - to)) / 2 >= length) @@ -4154,8 +4122,7 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident) /* Multibyte character overruns allowable length. */ if ((s - str) + charlen > len || memchr(s, 0, charlen) != NULL) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("incomplete multibyte character\n")); + libpq_append_error(conn, "incomplete multibyte character"); return NULL; } @@ -4172,8 +4139,7 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident) result = rp = (char *) malloc(result_size); if (rp == NULL) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); return NULL; } @@ -4337,8 +4303,7 @@ PQescapeByteaInternal(PGconn *conn, if (rp == NULL) { if (conn) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + libpq_append_error(conn, "out of memory"); return NULL; } diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 795500c593..f5324401ee 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -1277,4 +1277,19 @@ libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n) return dngettext(PG_TEXTDOMAIN("libpq"), msgid, msgid_plural, n); } +void +libpq_append_error(PGconn *conn, const char *fmt, ...) +{ + va_list args; + + Assert(fmt[strlen(fmt) - 1] != '\n'); + + va_start(args, fmt); + + appendPQExpBufferVA(&conn->errorMessage, libpq_gettext(fmt), args); + appendPQExpBufferChar(&conn->errorMessage, '\n'); + + va_end(args); +} + #endif /* ENABLE_NLS */ diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 1eb752a82d..21becf98cc 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -882,6 +882,8 @@ extern char *libpq_ngettext(const char *msgid, const char *msgid_plural, unsigne #define libpq_ngettext(s, p, n) ((n) == 1 ? (s) : (p)) #endif +extern void libpq_append_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 9256b426c1..9d994ac196 100644 --- a/src/interfaces/libpq/nls.mk +++ b/src/interfaces/libpq/nls.mk @@ -1,5 +1,5 @@ # 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_error:2 libpq_gettext pqInternalNotice:2 +GETTEXT_FLAGS = libpq_append_error:2 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 eb51e6d088..65621ec3b1 100644 --- a/src/interfaces/libpq/pqexpbuffer.c +++ b/src/interfaces/libpq/pqexpbuffer.c @@ -40,7 +40,7 @@ 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); +static bool appendPQExpBufferVA_internal(PQExpBuffer str, const char *fmt, va_list args) pg_attribute_printf(2, 0); /* @@ -250,7 +250,7 @@ printfPQExpBuffer(PQExpBuffer str, const char *fmt,...) { errno = save_errno; va_start(args, fmt); - done = appendPQExpBufferVA(str, fmt, args); + done = appendPQExpBufferVA_internal(str, fmt, args); va_end(args); } while (!done); } @@ -278,13 +278,30 @@ appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) { errno = save_errno; va_start(args, fmt); - done = appendPQExpBufferVA(str, fmt, args); + done = appendPQExpBufferVA_internal(str, fmt, args); va_end(args); } while (!done); } +void +appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args) +{ + int save_errno = errno; + bool done; + + if (PQExpBufferBroken(str)) + return; /* already failed */ + + /* Loop in case we have to retry after enlarging the buffer. */ + do + { + errno = save_errno; + done = appendPQExpBufferVA_internal(str, fmt, args); + } while (!done); +} + /* - * appendPQExpBufferVA + * appendPQExpBufferVA_internal * 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. @@ -293,7 +310,7 @@ appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) * when looping, in case the fmt contains "%m". */ static bool -appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args) +appendPQExpBufferVA_internal(PQExpBuffer str, const char *fmt, va_list args) { size_t avail; size_t needed; diff --git a/src/interfaces/libpq/pqexpbuffer.h b/src/interfaces/libpq/pqexpbuffer.h index efd652c80a..b8216b8a7b 100644 --- a/src/interfaces/libpq/pqexpbuffer.h +++ b/src/interfaces/libpq/pqexpbuffer.h @@ -157,6 +157,8 @@ extern void printfPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute */ extern void appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute_printf(2, 3); +extern void 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.1

view thread (14+ messages)  latest in thread

Message-ID: <7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com>
Permalink:  ../7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com/
Also on:    postgresql.org/message-id/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: peter.eisentraut@enterprisedb.com
  Subject: Re: libpq error message refactoring
  In-Reply-To: <7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox