Fix incorrect const qualification for tbm_add_tuples() and itemptr_to_uint64()
Hi Hackers,
I noticed a wrong const qualification:
```
void
tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bool recheck)
```
This "const" only protects "tids" itself from updating, which is
meaningless. I believe the real intention should be protecting the content
"tids" pointing to from updating.
This behavior can be easily proved by the compiler. If we add a line of
fake code in the function:
```
tids[0].ip_posid = 0;
```
With current "const ItemPointer tids", the compiler won't report any
problem. If we change to "const ItemPointerData *tids", the compiler will
raise an error due to the assignment to read-only variable.
I searched over the source tree, and found only one more occurrence in
itemptr_to_uint64(), so I fixed it as well.
Also, as I am touching tbm_add_tuples(), I did a tiny change that moved the
loop variable "i" into "for". Peter Eisentraut just did the same change in
formatting.c [1]https://git.postgresql.org/cgit/postgresql.git/commit/?id=03fbb0814c5015ab79e670ab97bb6a3349269e4b.
[1]: https://git.postgresql.org/cgit/postgresql.git/commit/?id=03fbb0814c5015ab79e670ab97bb6a3349269e4b
https://git.postgresql.org/cgit/postgresql.git/commit/?id=03fbb0814c5015ab79e670ab97bb6a3349269e4b
Best regards,
Chao Li (Evan)
---------------------
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v1-0001-Fix-incorrect-const-qualification-for-tbm_add_tup.patchapplication/octet-stream; name=v1-0001-Fix-incorrect-const-qualification-for-tbm_add_tup.patchDownload
From 78c228dda68cada702d67f433eb92583f3298933 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Wed, 29 Oct 2025 10:27:44 +0800
Subject: [PATCH v1] Fix incorrect const qualification for tbm_add_tuples() and
itemptr_to_uint64()
The parameters in tbm_add_tuples() and itemptr_to_uint64() were declared as
"const ItemPointer tids", which incorrectly applies const to the pointer itself
rather than to the data it points to. As a result, the contents referenced by
"tids" were not protected from modification, defeating the intended use of
const. Change these parameters to "const ItemPointerData *" so that the pointed-
to data are correctly treated as read-only.
Author: Chao Li <lic@highgo.com>
---
src/backend/access/gin/ginpostinglist.c | 2 +-
src/backend/nodes/tidbitmap.c | 5 ++---
src/include/nodes/tidbitmap.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c
index 48eadec87b0..6d8c8edc490 100644
--- a/src/backend/access/gin/ginpostinglist.c
+++ b/src/backend/access/gin/ginpostinglist.c
@@ -84,7 +84,7 @@
#define MaxBytesPerInteger 7
static inline uint64
-itemptr_to_uint64(const ItemPointer iptr)
+itemptr_to_uint64(const ItemPointerData *iptr)
{
uint64 val;
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index fac2ba5d0ca..57286863df0 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -364,15 +364,14 @@ tbm_free_shared_area(dsa_area *dsa, dsa_pointer dp)
* TBMIterateResult when any of these tuples are reported out.
*/
void
-tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
+tbm_add_tuples(TIDBitmap *tbm, const ItemPointerData *tids, int ntids,
bool recheck)
{
BlockNumber currblk = InvalidBlockNumber;
PagetableEntry *page = NULL; /* only valid when currblk is valid */
- int i;
Assert(tbm->iterating == TBM_NOT_ITERATING);
- for (i = 0; i < ntids; i++)
+ for (int i = 0; i < ntids; i++)
{
BlockNumber blk = ItemPointerGetBlockNumber(tids + i);
OffsetNumber off = ItemPointerGetOffsetNumber(tids + i);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index f54e61c7190..c24997d1c40 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -85,7 +85,7 @@ extern void tbm_free(TIDBitmap *tbm);
extern void tbm_free_shared_area(dsa_area *dsa, dsa_pointer dp);
extern void tbm_add_tuples(TIDBitmap *tbm,
- const ItemPointer tids, int ntids,
+ const ItemPointerData *tids, int ntids,
bool recheck);
extern void tbm_add_page(TIDBitmap *tbm, BlockNumber pageno);
--
2.39.5 (Apple Git-154)
On 29.10.25 04:11, Chao Li wrote:
I noticed a wrong const qualification:
```
void
tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bool recheck)
```This "const" only protects "tids" itself from updating, which is
meaningless. I believe the real intention should be protecting the
content "tids" pointing to from updating.This behavior can be easily proved by the compiler. If we add a line of
fake code in the function:
```
tids[0].ip_posid = 0;
```With current "const ItemPointer tids", the compiler won't report any
problem. If we change to "const ItemPointerData *tids", the compiler
will raise an error due to the assignment to read-only variable.I searched over the source tree, and found only one more occurrence in
itemptr_to_uint64(), so I fixed it as well.
I have committed this, and I also found a few more similarly confused
cases across the tree, which I also fixed.
Also, as I am touching tbm_add_tuples(), I did a tiny change that moved
the loop variable "i" into "for". Peter Eisentraut just did the same
change in formatting.c [1].
I don't know, let's leave unrelated changes for a separate patch.