Обсуждение: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.
Hi, I noticed that pg_logical_slot_get_changes_guts() assigns InvalidXLogRecPtr to the local variable upto_nchanges, even though it's not LSN variable (i.e., its type is int32, not XLogRecPtr). While this causes no functional issue since InvalidXLogRecPtr is defined as 0, it's semantically incorrect. I propose fixing this by setting upto_nchanges to 0 instead of InvalidXLogRecPtr. Attached is a patch implementing this change. Regards, -- Fujii Masao
Вложения
Agreed.
The definitions of upto_lsn and upyo_nchanges are different, so they should not be assigned the same form of value.
XLogRecPtr upto_lsn;
int32 upto_nchanges;
int32 upto_nchanges;
......
if (PG_ARGISNULL(1))
upto_lsn = InvalidXLogRecPtr;
else
upto_lsn = PG_GETARG_LSN(1);
if (PG_ARGISNULL(2))
upto_nchanges = InvalidXLogRecPtr;
else
upto_nchanges = PG_GETARG_INT32(2);
Thanks,
Steven
From: Fujii Masao <masao.fujii@gmail.com>
Sent: Wednesday, November 12, 2025 16:23
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.
Sent: Wednesday, November 12, 2025 16:23
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.
Hi,
I noticed that pg_logical_slot_get_changes_guts() assigns InvalidXLogRecPtr
to the local variable upto_nchanges, even though it's not LSN variable
(i.e., its type is int32, not XLogRecPtr). While this causes no functional issue
since InvalidXLogRecPtr is defined as 0, it's semantically incorrect.
I propose fixing this by setting upto_nchanges to 0 instead of
InvalidXLogRecPtr.
Attached is a patch implementing this change.
Regards,
--
Fujii Masao
I noticed that pg_logical_slot_get_changes_guts() assigns InvalidXLogRecPtr
to the local variable upto_nchanges, even though it's not LSN variable
(i.e., its type is int32, not XLogRecPtr). While this causes no functional issue
since InvalidXLogRecPtr is defined as 0, it's semantically incorrect.
I propose fixing this by setting upto_nchanges to 0 instead of
InvalidXLogRecPtr.
Attached is a patch implementing this change.
Regards,
--
Fujii Masao
Hi, On Wed, Nov 12, 2025 at 4:23 PM Fujii Masao <masao.fujii@gmail.com> wrote: > > Hi, > > I noticed that pg_logical_slot_get_changes_guts() assigns InvalidXLogRecPtr > to the local variable upto_nchanges, even though it's not LSN variable > (i.e., its type is int32, not XLogRecPtr). While this causes no functional issue > since InvalidXLogRecPtr is defined as 0, it's semantically incorrect. > > I propose fixing this by setting upto_nchanges to 0 instead of > InvalidXLogRecPtr. > Attached is a patch implementing this change. > > Regards, > > -- > Fujii Masao Good catch! I checked that no other similar misuses of InvalidXLogRecPtr assigned to non-LSN variables were found in logicalfuncs.c. -- Best, Xuneng
On Wed, Nov 12, 2025 at 6:14 PM Xuneng Zhou <xunengzhou@gmail.com> wrote: > Good catch! I checked that no other similar misuses of > InvalidXLogRecPtr assigned to non-LSN variables were found in > logicalfuncs.c. Thanks, Steven and Xuneng, for the reviews! I've pushed the patch. Regards, -- Fujii Masao