Increase repalloc_array() usage in buffile.c

Hi,

I was reading some buffile.c code, and I noticed some hand-rolled
versions of repalloc_array() (minus the multiplication check). Since we
already have one usage of repalloc_array() in buffile.c for
BufFile::files, I figured it would probably be alright if we used it in
the other two locations as well.

This seems like a good idea, especially with bfc5cea76d25
which hardens (re-)palloc_array against integer overflow.
I checked and could not find any other occurances in buffile.c that
should be included.

--
Sami Imseih
Amazon Web Services (AWS)

On 29 Jul 2026, at 21:27, Tristan Partin <tristan@partin.io> wrote:

I was reading some buffile.c code, and I noticed some hand-rolled
versions of repalloc_array() (minus the multiplication check). Since we
already have one usage of repalloc_array() in buffile.c for
BufFile::files, I figured it would probably be alright if we used it in
the other two locations as well.

+1 for the concept, but I wonder why these weren't covered by 1b105f9472bdb?

--
Daniel Gustafsson

I was reading some buffile.c code, and I noticed some hand-rolled
versions of repalloc_array() (minus the multiplication check). Since we
already have one usage of repalloc_array() in buffile.c for
BufFile::files, I figured it would probably be alright if we used it in
the other two locations as well.

+1 for the concept, but I wonder why these weren't covered by 1b105f9472bdb?

hmm, right. specifically 1b105f9472bdb bulk updated buffile.c. Only explanation
I see is they were missed in a large update.

--
Sami

On Wed, Jul 29, 2026 at 03:21:28PM -0500, Sami Imseih wrote:

hmm, right. specifically 1b105f9472bdb bulk updated buffile.c. Only explanation
I see is they were missed in a large update.

If you are interested in more work in this area, I think that we still
have a couple of holes in contrib/, and even more in the backend code.
Perhaps a LLM scan would help.

The reason why these holes exist is that the patch series of
1b105f9472bdb was quite large, so I have been focusing mainly on
making sure that all the suggested changes were right instead of
trying to fix all of them. The long term picture is that we should do
the switch to do more type enforcements, and that this work is
incremental.

Saying all that, the proposed patch makes sense, and this is
incremental work, so I am going to apply it.

Thanks for caring. :)
--
Michael

On Fri, Jul 31, 2026 at 08:42:21AM +0900, Michael Paquier wrote:

The reason why these holes exist is that the patch series of
1b105f9472bdb was quite large, so I have been focusing mainly on
making sure that all the suggested changes were right instead of
trying to fix all of them. The long term picture is that we should do
the switch to do more type enforcements, and that this work is
incremental.

Saying all that, the proposed patch makes sense, and this is
incremental work, so I am going to apply it.

By the way, just mentioning two of these holes, using the following
command in src/backend/storage/file/:
$ git grep -A 2 "alloc(" | grep sizeof
fd.c: fd = (int *) palloc(size * sizeof(int));
fd.c: fd = (int *) repalloc(fd, size * sizeof(int));

Switched these extra two spots, while on it, and applied the result.
--
Michael

On Thu Jul 30, 2026 at 6:42 PM CDT, Michael Paquier wrote:

On Wed, Jul 29, 2026 at 03:21:28PM -0500, Sami Imseih wrote:

hmm, right. specifically 1b105f9472bdb bulk updated buffile.c. Only explanation
I see is they were missed in a large update.

If you are interested in more work in this area, I think that we still
have a couple of holes in contrib/, and even more in the backend code.
Perhaps a LLM scan would help.

The reason why these holes exist is that the patch series of
1b105f9472bdb was quite large, so I have been focusing mainly on
making sure that all the suggested changes were right instead of
trying to fix all of them. The long term picture is that we should do
the switch to do more type enforcements, and that this work is
incremental.

Saying all that, the proposed patch makes sense, and this is
incremental work, so I am going to apply it.

Thanks for caring. :)

Let me see what I can come up with. As a committer, how would you like
the patches? I could do file by file, subsystem by subsystem, or just
one large single patch? I don't mind putting in the work to translate
all realloc calls where it makes sense, but I would like to make sure
I organize it in such a way to minimize work for you.

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

On Fri, Jul 31, 2026 at 03:54:33AM +0000, Tristan Partin wrote:

Let me see what I can come up with. As a committer, how would you like
the patches? I could do file by file, subsystem by subsystem, or just
one large single patch? I don't mind putting in the work to translate
all realloc calls where it makes sense, but I would like to make sure
I organize it in such a way to minimize work for you.

All of these are mechanical changes, and it is possible to do a set of
`git add` to split these across sub-directories. So a single huge
patch does not change much compared to many small ones from my
perspective. So your call.
--
Michael

On Fri Jul 31, 2026 at 2:35 AM CDT, Michael Paquier wrote:

On Fri, Jul 31, 2026 at 03:54:33AM +0000, Tristan Partin wrote:

Let me see what I can come up with. As a committer, how would you like
the patches? I could do file by file, subsystem by subsystem, or just
one large single patch? I don't mind putting in the work to translate
all realloc calls where it makes sense, but I would like to make sure
I organize it in such a way to minimize work for you.

All of these are mechanical changes, and it is possible to do a set of
`git add` to split these across sub-directories. So a single huge
patch does not change much compared to many small ones from my
perspective. So your call.

I started working on this yesterday. One pattern that became pretty
noticeable was:

buf = MemoryContextAlloc(context, elements * sizeof(*buf));
...
buf = repalloc(buf, new_elements * sizeof(*buf));

I wonder if we should add MemoreContextAllocArray() and friends, similar
to palloc_array().

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

On Fri Jul 31, 2026 at 7:35 AM UTC, Michael Paquier wrote:

On Fri, Jul 31, 2026 at 03:54:33AM +0000, Tristan Partin wrote:

Let me see what I can come up with. As a committer, how would you like
the patches? I could do file by file, subsystem by subsystem, or just
one large single patch? I don't mind putting in the work to translate
all realloc calls where it makes sense, but I would like to make sure
I organize it in such a way to minimize work for you.

All of these are mechanical changes, and it is possible to do a set of
`git add` to split these across sub-directories. So a single huge
patch does not change much compared to many small ones from my
perspective. So your call.

Here are 3 patches that hope to modernize most vanilla palloc() calls
where appropriate.

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

On Sat, Aug 08, 2026 at 05:19:59AM +0000, Tristan Partin wrote:

Here are 3 patches that hope to modernize most vanilla palloc() calls
where appropriate.

That took some time to go through. Some portions of the patch did not
apply due to some.. cough.. Recent commits. Please feel free to check
the tree around src/pl/ for example.

-   arrayKeyData = (ScanKey) palloc(numArrayKeyData * sizeof(ScanKeyData));
+   arrayKeyData = (ScanKey) palloc_array(ScanKeyData, numArrayKeyData);
[...]
-           first_sub_key = (ScanKey)
-               palloc(list_length(rc->opnos) * sizeof(ScanKeyData));
+           first_sub_key = (ScanKey) palloc_array(ScanKeyData, list_length(rc->opnos));
[...]
-       setopstate->sortKeys = (SortSupport)
-           palloc0(nkeys * sizeof(SortSupportData));
+       setopstate->sortKeys = (SortSupport) palloc0_array(SortSupportData, nkeys);
[...]
-   clauses = (MergeJoinClause) palloc0(nClauses * sizeof(MergeJoinClauseData));
+   clauses = (MergeJoinClause) palloc0_array(MergeJoinClauseData, nClauses);

No need for casts in these ones.

-               winobj->notnull_info[argno] = palloc0(newsize);
+               winobj->notnull_info[argno] = palloc0_array(uint8, newsize);
[...]
-                   repalloc0(winobj->notnull_info[argno], oldsize, newsize);
+                   repalloc0_array(winobj->notnull_info[argno], uint8, oldsize, newsize);

Not sure if these are worth changing.

-   argv = (char **) palloc0_array(char *, row->nfields);
+   argv = palloc0_array(char *, row->nfields);

In pl_gram.y. Cast that was indeed not required.

There are quite a few places where we assume a number of bytes while
the code enforces char, for code paths where sizeof(char) is not used.
I am having cold feet on these ones as it hides some of the original
intention of the code (noted 50~55 changes or so related to this
pattern). Something similar can be said with pgcrypto, as well. So I
have left these off. The other changes refer generally to more
complex structures.

And 51c43a5dbd86 it is. This kind of work is incremental, and we have
many much more things that could be done. Patches are always welcome,
of course.
--
Michael