> On Mar 9, 2026, at 08:40, Chao Li <[email protected]> wrote: > > > >> On Mar 7, 2026, at 03:33, Masahiko Sawada <[email protected]> wrote: >> >> Hi, >> >> On Tue, Mar 3, 2026 at 5:31 PM Chao Li <[email protected]> wrote: >>> >>> >>> >>>> On Dec 25, 2025, at 11:34, Chao Li <[email protected]> wrote: >>>> >>>> >>>> >>>>> On Dec 25, 2025, at 11:12, Chao Li <[email protected]> wrote: >>>>> >>>>> Hi Hackers, >>>>> >>>>> I noticed this error while working on [1]. >>>>> >>>>> In BufFile, the fields is claimed as an array: >>>>> ``` >>>>> struct BufFile >>>>> { >>>>> File *files; /* palloc'd array with numFiles entries */ >>>>> ``` >>>>> >>>>> However, it’s allocated by palloc_object(): >>>>> ``` >>>>> file->files = palloc_object(File); >>>>> ``` >>>>> >>>>> And reallocated by repalloc(): >>>>> ``` >>>>> file->files = (File *) repalloc(file->files, >>>>> (file->numFiles + 1) * sizeof(File)); >>>>> ``` >>>>> >>>>> This trivial patch just changes to use palloc_array/repalloc_array, which >>>>> makes the intent clearer. >>>>> >>>>> Best regards, >>>>> -- >>>>> Chao Li (Evan) >>>>> HighGo Software Co., Ltd. >>>>> https://www.highgo.com/ >>>>> >>>>> >>>>> >>>>> >>>>> <v1-0001-Use-palloc_array-repalloc_array-for-BufFile-file-.patch> >>>> >>>> >>>> Sorry for missing the reference of [1]: >>>> >>>> [1] https://postgr.es/m/[email protected] >>>> >>>> Best regards, >>>> -- >>>> Chao Li (Evan) >>>> HighGo Software Co., Ltd. >>>> https://www.highgo.com/ >>>> >>> >>> PFA v2: >>> * Rebased >>> * Updated the commit message >> >> I've reviewed the v2 patch and here is a comment: >> >> - file->files = palloc_object(File); >> + file->files = palloc_array(File, 1); >> >> I'm not a fan of this change. This change looks like trying to >> distinguish allocated memory by palloc_object() and palloc_array() >> even though underlying memory is identical. I'm concerned about this >> change creating unnecessary coding conventions. >> > > Hi Masahiho-san, > > Thank you so much for taking the time to review this patch. > > Actually, this change was my original motivation for creating the patch. When > I first read the code, I was confused why the field was named with the plural > “files" even though it was allocated with palloc_object(). After reading > further, I saw that the memory is later enlarged with repalloc, so it became > clear that file->files is really meant to be an array. > > To me, the allocation method should reflect the actual meaning of the object. > Here, file->files is an array, no matter how many elements it currently > contains. Even if it only has one element at the moment, semantically it is > still an array. > > By “creating unnecessary coding conventions”, are you concerned that this > change might encourage people to always use palloc_array(type, 1) when > allocating a single object? That concern is understandable, but I think it > should depend on the real semantics. If the memory is for a single object, > then of course palloc_array() should be wrong. > > Given the nature of C, a "File *" can point either to a single File object or > to a File array. In that sense, palloc_array() makes it explicit that this > pointer is intended to represent an array, while palloc_object() suggests a > single object. So I think changing this to palloc_array() improves > readability in this case. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/
Hi Masahiko-san,
Would you mind sharing any further thoughts on this?
While working further on this patch, I noticed this code in ginget.c:
```
key->requiredEntries = palloc(1 * sizeof(GinScanEntry));
```
To me, this seems to use * 1 to make it explicit that key->requiredEntries is
intended as an array, even though it currently contains only one element. That
feels similar to what I was trying to express with: " file->files =
palloc_array(File, 1);”. But if you still feel this is not a good change, I’m
happy to revert it.
Also, while reviewing recent patches, I found a number of additional
palloc/repalloc call sites that could be converted to the newer helpers, so I
expanded the scope of this patch a bit. To avoid making it too large, I
included only about 1/3 of the cases I found in this version. If this patch
moves forward, I’d be happy to clean up the remaining occurrences separately.
PFA v3.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v3-0001-Use-array-allocation-helpers-in-more-places.patch
Description: Binary data
