Обсуждение: Trivial Fix: use palloc_array/repalloc_array for BufFile file arrays
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));
(file->numFiles + 1) * sizeof(File));
```
This trivial patch just changes to use palloc_array/repalloc_array, which makes the intent clearer.
Вложения
> 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/
> 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
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Вложения
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.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
> 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 why
thefield was named with the plural “files" even though it was allocated with palloc_object(). After reading further, I
sawthat 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
howmany 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
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/