Incorrect assert in libpqwalreceiver

Started by Jacob Brazeal10 months ago2 messages
#1Jacob Brazeal
jacob.brazeal@gmail.com
1 attachment(s)

Hello hackers,

The libpqrcv_connect function asserts 'Assert(i < sizeof(keys))', where
keys is declared as const char *keys[6];.

However, sizeof(keys) is not the correct way to check the array length (on
my system, for example, it's 48 = 6 * 8 at this callsite, not 6.)

I attached a patch to fix the assert, but I suppose we could also just
remove the assert altogether, since it hasn't been doing anything for at
least 8 years.

Regards,
Jacob

Attachments:

v1-length-assert.patchapplication/octet-stream; name=v1-length-assert.patchDownload
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 1b158c9d288..c650935ef5d 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -208,7 +208,7 @@ libpqrcv_connect(const char *conninfo, bool replication, bool logical,
 	keys[++i] = NULL;
 	vals[i] = NULL;
 
-	Assert(i < sizeof(keys));
+	Assert(i < lengthof(keys));
 
 	conn = palloc0(sizeof(WalReceiverConn));
 	conn->streamConn = PQconnectStartParams(keys, vals,
#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Jacob Brazeal (#1)
Re: Incorrect assert in libpqwalreceiver

On 09/03/2025 10:09, Jacob Brazeal wrote:

The libpqrcv_connect function asserts 'Assert(i < sizeof(keys))', where
keys is declared as const char *keys[6];.

However, sizeof(keys) is not the correct way to check the array length
(on my system, for example, it's 48 = 6 * 8 at this callsite, not 6.)

I attached a patch to fix the assert, but I suppose we could also just
remove the assert altogether, since it hasn't been doing anything for at
least 8 years.

Committed, thanks!

I think it's still valuable; it's an easy mistake to make, to add a
parameter and forget to increase the array size.

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