Message-ID: <09323e6a-4743-4be2-9d7f-74b86e4dac64@iki.fi>
Date: Tue, 8 Apr 2025 22:41:31 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: pgsql: Make cancel request keys longer
To: Peter Eisentraut <peter@eisentraut.org>,
 pgsql-committers@lists.postgresql.org
References: <E1tzyNB-002IXa-0i@gemulon.postgresql.org>
 <61be9e31-7b7d-49d5-bc11-721800d89d64@eisentraut.org>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <61be9e31-7b7d-49d5-bc11-721800d89d64@eisentraut.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/09323e6a-4743-4be2-9d7f-74b86e4dac64%40iki.fi>
Precedence: bulk

On 08/04/2025 20:06, Peter Eisentraut wrote:
> On 02.04.25 15:43, Heikki Linnakangas wrote:
>> Make cancel request keys longer
> 
> This patch changed the signature of ProcSignal()
> 
> -ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
> +ProcSignalInit(char *cancel_key, int cancel_key_len)
> 
> but did not update the caller in auxprocess.c:
> 
> ProcSignalInit(false, 0);
> 
> This gives a warning with clang.

Good catch. I wonder why the cirrus CI didn't complain, it has a step to 
check for warnings with clang.

> While I was looking at this, I suggest to make the first argument void 
> *.  This is consistent for passing binary data.

Ok, sure.

> Also, I wonder why MyCancelKeyLength is of type uint8 rather than 
> something more mundane like int.  There doesn't seem to be any API 
> reason for this type.

Agreed. The cancel key length is documented to be at most 256 bytes, but 
that's more of a coincidence, nothing depends on that variable being uint8.

> See attached patch for possible changes.

Looks good to me. I can commit these tomorrow, or feel free to do it 
yourself too.

Thank you!

-- 
Heikki Linnakangas
Neon (https://neon.tech)