On 09/05/2025 01:28, Jacob Champion wrote:
On Thu, May 8, 2025 at 12:11 PM Heikki Linnakangas <hlinn...@iki.fi> wrote:
Polished this up a tiny bit, and committed.
Thanks! I think the uint8->int change for cancel_key_len is more than
just cosmetic; it most likely fixes a bug where a key size of 256
wrapped around to 0. I'll double-check that this fixes that later;
I've gotten side-tracked from the protocol stuff a bit.
True, although I'm pretty sure you'd fail the later cross-check that the
whole message was consumed. ("message contents do not agree with length
in message type"). But it's fixed now in any case.
While I have you, though, is the following just a really complicated
way to say `msgLength - 4`, or is there some other reason to do the
pointer math?
cancel_key_len = 5 + msgLength - (conn->inCursor - conn->inStart);
Yes, it amounts to 'msgLength - 4'. I agree it looks pretty obscure. The
way to read it is:
/* full length of the message, including the type code byte and the
length field itself */
fullMsgLength = 5 + msgLength;
/* number of bytes consumed from the message so far */
lengthConsumed = (conn->inCursor - conn->inStart);
/* the cancel key consumes all the remaining bytes of the message */
cancel_key_len = fullMsgLength - lengthConsumed;
It didn't occur to me that you could write it simply as 'msgLength - 4'.
That depends on knowing that the preceding fields are exactly 4 bytes
long, but that's clear enough if we just add a comment on that, see
attached.
--
Heikki Linnakangas
Neon (https://neon.tech)
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index beb1c889aad..551276e9011 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1544,7 +1544,11 @@ getBackendKeyData(PGconn *conn, int msgLength)
if (pqGetInt(&(conn->be_pid), 4, conn))
return EOF;
- cancel_key_len = 5 + msgLength - (conn->inCursor - conn->inStart);
+ /*
+ * The pid consumed 4 bytes, and the cancellation key consumes the rest of
+ * the message.
+ */
+ cancel_key_len = msgLength - 4;
conn->be_cancel_key = malloc(cancel_key_len);
if (conn->be_cancel_key == NULL)
