Re: Trivial Fix: use palloc_array/repalloc_array for BufFile file arrays - Mailing list pgsql-hackers

> On Mar 9, 2026, at 08:40, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
>> On Mar 7, 2026, at 03:33, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>
>> Hi,
>>
>> On Tue, Mar 3, 2026 at 5:31 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>>
>>>
>>>> On Dec 25, 2025, at 11:34, Chao Li <li.evan.chao@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Dec 25, 2025, at 11:12, Chao Li <li.evan.chao@gmail.com> 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/aUStrqoOCDRFAq1M@paquier.xyz
>>>>
>>>> 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
whythe 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
bean array. 
>
> To me, the allocation method should reflect the actual meaning of the object. Here, file->files is an array, no
matterhow many elements it currently contains. Even if it only has one element at the moment, semantically it is still
anarray. 
>
> 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
thereal 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
singleobject. 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
currentlycontains 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
tothe newer helpers, so I expanded the scope of this patch a bit. To avoid making it too large, I included only about
1/3of the cases I found in this version. If this patch moves forward, I’d be happy to clean up the remaining
occurrencesseparately. 

PFA v3.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/