Обсуждение: Fix incorrect const qualification for tbm_add_tuples() and itemptr_to_uint64()

Поиск
Список
Период
Сортировка

Fix incorrect const qualification for tbm_add_tuples() and itemptr_to_uint64()

От
Chao Li
Дата:
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].


Best regards,
Chao Li (Evan)
---------------------
HighGo Software Co., Ltd.
https://www.highgo.com/
Вложения

Re: Fix incorrect const qualification for tbm_add_tuples() and itemptr_to_uint64()

От
Peter Eisentraut
Дата:
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.