On 2024-Jan-29, Jelte Fennema-Nio wrote:
> On Mon, 29 Jan 2024 at 12:44, Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
> > Thanks! I committed 0001 now. I also renamed the new
> > pq_parse_int_param to pqParseIntParam, for consistency with other
> > routines there. Please rebase the other patches.
>
> Awesome! Rebased, and renamed pq_release_conn_hosts to
> pqReleaseConnHosts for the same consistency reasons.
Thank you, looks good.
I propose the following minor/trivial fixes over your initial 3 patches.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"I can't go to a restaurant and order food because I keep looking at the
fonts on the menu. Five minutes later I realize that it's also talking
about food" (Donald Knuth)
>From 92ca8dc2739a777ff5a0df990d6e9818c5729ac5 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Fri, 2 Feb 2024 10:57:02 +0100
Subject: [PATCH 1/4] pgindent
---
src/interfaces/libpq/fe-cancel.c | 14 +++++++-------
src/interfaces/libpq/libpq-fe.h | 17 +++++++++--------
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/interfaces/libpq/fe-cancel.c b/src/interfaces/libpq/fe-cancel.c
index 7416791d9f..d75c9628e7 100644
--- a/src/interfaces/libpq/fe-cancel.c
+++ b/src/interfaces/libpq/fe-cancel.c
@@ -137,7 +137,7 @@ oom_error:
* Returns 1 if successful 0 if not.
*/
int
-PQcancelSend(PGcancelConn * cancelConn)
+PQcancelSend(PGcancelConn *cancelConn)
{
if (!cancelConn || cancelConn->conn.status == CONNECTION_BAD)
return 1;
@@ -157,7 +157,7 @@ PQcancelSend(PGcancelConn * cancelConn)
* Poll a cancel connection. For usage details see PQconnectPoll.
*/
PostgresPollingStatusType
-PQcancelPoll(PGcancelConn * cancelConn)
+PQcancelPoll(PGcancelConn *cancelConn)
{
PGconn *conn = (PGconn *) cancelConn;
int n;
@@ -249,7 +249,7 @@ PQcancelPoll(PGcancelConn * cancelConn)
* Get the status of a cancel connection.
*/
ConnStatusType
-PQcancelStatus(const PGcancelConn * cancelConn)
+PQcancelStatus(const PGcancelConn *cancelConn)
{
return PQstatus((const PGconn *) cancelConn);
}
@@ -260,7 +260,7 @@ PQcancelStatus(const PGcancelConn * cancelConn)
* Get the socket of the cancel connection.
*/
int
-PQcancelSocket(const PGcancelConn * cancelConn)
+PQcancelSocket(const PGcancelConn *cancelConn)
{
return PQsocket((const PGconn *) cancelConn);
}
@@ -271,7 +271,7 @@ PQcancelSocket(const PGcancelConn * cancelConn)
* Get the socket of the cancel connection.
*/
char *
-PQcancelErrorMessage(const PGcancelConn * cancelConn)
+PQcancelErrorMessage(const PGcancelConn *cancelConn)
{
return PQerrorMessage((const PGconn *) cancelConn);
}
@@ -283,7 +283,7 @@ PQcancelErrorMessage(const PGcancelConn * cancelConn)
* request.
*/
void
-PQcancelReset(PGcancelConn * cancelConn)
+PQcancelReset(PGcancelConn *cancelConn)
{
pqClosePGconn((PGconn *) cancelConn);
cancelConn->conn.status = CONNECTION_STARTING;
@@ -299,7 +299,7 @@ PQcancelReset(PGcancelConn * cancelConn)
* Closes and frees the cancel connection.
*/
void
-PQcancelFinish(PGcancelConn * cancelConn)
+PQcancelFinish(PGcancelConn *cancelConn)
{
PQfinish((PGconn *) cancelConn);
}
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 857ba54d94..851e549355 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -329,17 +329,18 @@ extern PostgresPollingStatusType PQresetPoll(PGconn
*conn);
extern void PQreset(PGconn *conn);
/* Create a PGcancelConn that's used to cancel a query on the given PGconn */
-extern PGcancelConn * PQcancelConn(PGconn *conn);
+extern PGcancelConn *PQcancelConn(PGconn *conn);
+
/* issue a blocking cancel request */
-extern int PQcancelSend(PGcancelConn * conn);
+extern int PQcancelSend(PGcancelConn *conn);
/* issue or poll a non-blocking cancel request */
-extern PostgresPollingStatusType PQcancelPoll(PGcancelConn * cancelConn);
-extern ConnStatusType PQcancelStatus(const PGcancelConn * cancelConn);
-extern int PQcancelSocket(const PGcancelConn * cancelConn);
-extern char *PQcancelErrorMessage(const PGcancelConn * cancelConn);
-extern void PQcancelReset(PGcancelConn * cancelConn);
-extern void PQcancelFinish(PGcancelConn * cancelConn);
+extern PostgresPollingStatusType PQcancelPoll(PGcancelConn *cancelConn);
+extern ConnStatusType PQcancelStatus(const PGcancelConn *cancelConn);
+extern int PQcancelSocket(const PGcancelConn *cancelConn);
+extern char *PQcancelErrorMessage(const PGcancelConn *cancelConn);
+extern void PQcancelReset(PGcancelConn *cancelConn);
+extern void PQcancelFinish(PGcancelConn *cancelConn);
/* request a cancel structure */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 91433d439b..9ffb169e9d 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1760,6 +1760,7 @@ PG_Locale_Strategy
PG_Lock_Status
PG_init_t
PGcancel
+PGcancelConn
PGcmdQueueEntry
PGconn
PGdataValue
--
2.39.2
>From 2fb777f47288815a370a15c7c17f39496b64c4f5 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Fri, 2 Feb 2024 13:16:51 +0100
Subject: [PATCH 2/4] Add missing period
---
doc/src/sgml/libpq.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 9808e67865..67f6378ba8 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -6093,7 +6093,7 @@ int PQcancelSend(PGcancelConn *conn);
<para>
The request is made over the given
<structname>PGcancelConn</structname>,
- which needs to be created with <xref linkend="libpq-PQcancelConn"/>
+ which needs to be created with <xref linkend="libpq-PQcancelConn"/>.
The return value of <xref linkend="libpq-PQcancelSend"/>
is 1 if the cancel request was successfully
dispatched and 0 if not. If it was unsuccessful, the error message can
be
--
2.39.2
>From c2a6bc14174a1e7a8a4f1508a5c30ee0d30d5f47 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Fri, 2 Feb 2024 13:17:10 +0100
Subject: [PATCH 3/4] Add missing 'const' decorator in documentation
---
doc/src/sgml/libpq.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 67f6378ba8..8648379f8d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -6166,7 +6166,7 @@ ConnStatusType PQcancelStatus(const PGcancelConn *conn);
A version of <xref linkend="libpq-PQsocket"/> that can be used for
cancellation connections.
<synopsis>
-int PQcancelSocket(PGcancelConn *conn);
+int PQcancelSocket(const PGcancelConn *conn);
</synopsis>
</para>
</listitem>
--
2.39.2
>From ba6cc0ba4d387015d64a98a37eb72188191c3672 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Fri, 2 Feb 2024 13:17:25 +0100
Subject: [PATCH 4/4] Wording, whitespace changes
---
doc/src/sgml/libpq.sgml | 8 ++++----
src/interfaces/libpq/fe-cancel.c | 9 +++------
src/interfaces/libpq/fe-connect.c | 12 ++++++------
3 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 8648379f8d..81b4028381 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -6052,7 +6052,7 @@ PGcancelConn *PQcancelConn(PGconn *conn);
connection. A cancel request can be sent over this connection in a
blocking manner using <xref linkend="libpq-PQcancelSend"/> and in a
non-blocking manner using <xref linkend="libpq-PQcancelPoll"/>.
- The return value should can be passed to <xref
linkend="libpq-PQcancelStatus"/>,
+ The return value can be passed to <xref linkend="libpq-PQcancelStatus"/>
to check if the <structname>PGcancelConn</structname> object was
created successfully. The <structname>PGcancelConn</structname> object
is an opaque structure that is not meant to be accessed directly by the
@@ -6150,9 +6150,9 @@ ConnStatusType PQcancelStatus(const PGcancelConn *conn);
returns <symbol>CONNECTION_OK</symbol> for a
<structname>PGcancelConn</structname>
it means that that the dispatch of the cancel request has completed
(although
this is no promise that the query was actually canceled) and that the
- connection is now closed. While a <symbol>CONNECTION_OK</symbol> result
- for <structname>PGconn</structname> means that queries can be sent over
- the connection.
+ cancel connection is now closed, while a <symbol>CONNECTION_OK</symbol>
+ result for <structname>PGconn</structname> means that queries can be
+ sent over the connection.
</para>
</listitem>
diff --git a/src/interfaces/libpq/fe-cancel.c b/src/interfaces/libpq/fe-cancel.c
index d75c9628e7..6420384be7 100644
--- a/src/interfaces/libpq/fe-cancel.c
+++ b/src/interfaces/libpq/fe-cancel.c
@@ -51,7 +51,6 @@ PQcancelConn(PGconn *conn)
return (PGcancelConn *) cancelConn;
}
-
/*
* Indicate that this connection is used to send a cancellation
*/
@@ -223,20 +222,19 @@ PQcancelPoll(PGcancelConn *cancelConn)
#endif
/*
- * We don't expect any data, only connection closure. So if we strangly
do
+ * We don't expect any data, only connection closure. So if we
strangely do
* receive some data we consider that an error.
*/
if (n > 0)
{
-
libpq_append_conn_error(conn, "received unexpected response
from server");
conn->status = CONNECTION_BAD;
return PGRES_POLLING_FAILED;
}
/*
- * Getting here means that we received an EOF. Which is what we were
- * expecting. The cancel request has completed.
+ * Getting here means that we received an EOF, which is what we were
+ * expecting -- the cancel request has completed.
*/
cancelConn->conn.status = CONNECTION_OK;
resetPQExpBuffer(&conn->errorMessage);
@@ -304,7 +302,6 @@ PQcancelFinish(PGcancelConn *cancelConn)
PQfinish((PGconn *) cancelConn);
}
-
/*
* PQgetCancel: get a PGcancel structure corresponding to a connection.
*
diff --git a/src/interfaces/libpq/fe-connect.c
b/src/interfaces/libpq/fe-connect.c
index 4add35ec5c..4f9b2182db 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -618,9 +618,9 @@ pqDropServerData(PGconn *conn)
conn->write_err_msg = NULL;
/*
- * Cancel connections should save their be_pid and be_key across
- * PQcancelReset invocations. Otherwise they would not have access to
the
- * secret token of the connection they are supposed to cancel anymore.
+ * Cancel connections need to retain their be_pid and be_key across
+ * PQcancelReset invocations, otherwise they would not have access to
the
+ * secret token of the connection they are supposed to cancel.
*/
if (!conn->cancelRequest)
{
@@ -648,8 +648,8 @@ pqDropServerData(PGconn *conn)
* PQconnectStart or PQconnectStartParams (which differ in the same way as
* PQconnectdb and PQconnectdbParams) and PQconnectPoll.
*
- * Internally, the static functions pqConnectDBStart, pqConnectDBComplete
- * are part of the connection procedure.
+ * The non-exported functions pqConnectDBStart, pqConnectDBComplete are
+ * part of the connection procedure implementation.
*/
/*
@@ -2358,7 +2358,7 @@ pqConnectDBStart(PGconn *conn)
* anything else looks at it.)
*
* Cancel requests are special though, they should only try one host and
- * address. These fields have already set up in PQcancelConn. So leave
+ * address, and these fields have already set up in PQcancelConn, so
leave
* these fields alone for cancel requests.
*/
if (!conn->cancelRequest)
--
2.39.2
