Content-Type: multipart/mixed; boundary="------------5U7uZecrOwahUq2jWaup4x8N"
Message-ID: <a92c439b-6f6d-4460-b5ec-1aacd80a866b@iki.fi>
Date: Fri, 9 May 2025 09:37:58 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: pgsql: Make cancel request keys longer
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Peter Eisentraut <peter@eisentraut.org>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: <E1tzyNB-002IXa-0i@gemulon.postgresql.org>
 <61be9e31-7b7d-49d5-bc11-721800d89d64@eisentraut.org>
 <09323e6a-4743-4be2-9d7f-74b86e4dac64@iki.fi>
 <34d5d731-06cc-43af-88cd-b3a4c5c8d9df@iki.fi>
 <2afbd9c6-51b7-450f-9ae1-61e552368963@eisentraut.org>
 <4bd8421a-50ad-4169-a096-99247c2f563c@iki.fi>
 <9399dc5d-5575-483e-8e23-af6be79385c8@iki.fi>
 <52d648b1-9aca-43d1-a9e4-a6a556410f41@iki.fi>
 <CAOYmi+kJcKkbe3Ng7=jYOz7V2gYEvdNXKESKbmuW=6N7=243tg@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: 
 <CAOYmi+kJcKkbe3Ng7=jYOz7V2gYEvdNXKESKbmuW=6N7=243tg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/a92c439b-6f6d-4460-b5ec-1aacd80a866b%40iki.fi>
Precedence: bulk

This is a multi-part message in MIME format.
--------------5U7uZecrOwahUq2jWaup4x8N
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 09/05/2025 01:28, Jacob Champion wrote:
> On Thu, May 8, 2025 at 12:11 PM Heikki Linnakangas <hlinnaka@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)

--------------5U7uZecrOwahUq2jWaup4x8N
Content-Type: text/x-patch; charset=UTF-8;
 name="simplify-cancel_key_len-math.patch"
Content-Disposition: attachment; filename="simplify-cancel_key_len-math.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9pbnRlcmZhY2VzL2xpYnBxL2ZlLXByb3RvY29sMy5jIGIvc3Jj
L2ludGVyZmFjZXMvbGlicHEvZmUtcHJvdG9jb2wzLmMKaW5kZXggYmViMWM4ODlhYWQuLjU1
MTI3NmU5MDExIDEwMDY0NAotLS0gYS9zcmMvaW50ZXJmYWNlcy9saWJwcS9mZS1wcm90b2Nv
bDMuYworKysgYi9zcmMvaW50ZXJmYWNlcy9saWJwcS9mZS1wcm90b2NvbDMuYwpAQCAtMTU0
NCw3ICsxNTQ0LDExIEBAIGdldEJhY2tlbmRLZXlEYXRhKFBHY29ubiAqY29ubiwgaW50IG1z
Z0xlbmd0aCkKIAlpZiAocHFHZXRJbnQoJihjb25uLT5iZV9waWQpLCA0LCBjb25uKSkKIAkJ
cmV0dXJuIEVPRjsKIAotCWNhbmNlbF9rZXlfbGVuID0gNSArIG1zZ0xlbmd0aCAtIChjb25u
LT5pbkN1cnNvciAtIGNvbm4tPmluU3RhcnQpOworCS8qCisJICogVGhlIHBpZCBjb25zdW1l
ZCA0IGJ5dGVzLCBhbmQgdGhlIGNhbmNlbGxhdGlvbiBrZXkgY29uc3VtZXMgdGhlIHJlc3Qg
b2YKKwkgKiB0aGUgbWVzc2FnZS4KKwkgKi8KKwljYW5jZWxfa2V5X2xlbiA9IG1zZ0xlbmd0
aCAtIDQ7CiAKIAljb25uLT5iZV9jYW5jZWxfa2V5ID0gbWFsbG9jKGNhbmNlbF9rZXlfbGVu
KTsKIAlpZiAoY29ubi0+YmVfY2FuY2VsX2tleSA9PSBOVUxMKQo=

--------------5U7uZecrOwahUq2jWaup4x8N--