| From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix incorrect const qualification for tbm_add_tuples() and itemptr_to_uint64() |
| Date: | 2025-10-30 10:31:42 |
| Message-ID: | 964a4a44-5689-427f-a66a-9584c54eb1e1@eisentraut.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David Rowley | 2025-10-30 10:36:25 | Re: another autovacuum scheduling thread |
| Previous Message | Chao Li | 2025-10-30 10:28:53 | Re: display hot standby state in psql prompt |