Re: Consistently use palloc_object() and palloc_array()

From: David Geier <geidav(dot)pg(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Consistently use palloc_object() and palloc_array()
Date: 2025-12-05 12:53:37
Message-ID: accd2714-19ea-43a6-b4ea-29ae5416431c@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 05.12.2025 08:41, Michael Paquier wrote:
> On Thu, Dec 04, 2025 at 10:31:41AM +0100, David Geier wrote:
>> Attached are the two patches, rebased on latest master.
>>
>> The first one contains all changes that either result in no changes to
>> the disassembly, or only in changes due to the line counts; because
>> elog.h makes use of __LINE__.
>
> Thanks. That's a lot to digest.
>
> I have generated some reports by comparing builds of HEAD and
> HEAD+0001 to indeed note that a lot of noise is caused by the line
> number changes. And then, I have begun looking at contrib/ to start
> with something as I had a bit of time today. One part which we need
> to be careful about is that the allocations map with the actual
> declarations, and I'm doubting my compiler to detect all of them, so
> visual check for each path it is...
>
> - void *out = palloc(sizeof(float4KEY));
> + void *out = palloc_object(float4KEY);
>
> btree_gist/ has a bunch of these, which I guess don't really matter in
> terms of type safety.
>
> - sv = palloc(sizeof(bytea *) * (maxoff + 1));
> + sv = palloc_array(bytea *, (maxoff + 1));
>
> This one also in gbt_var_picksplit(at)btree_utils_var(dot)c(dot) We have a
> GBT_VARKEY.
>
> + keys = (char **) palloc_array(char *, key_count);
> + values = (char **) palloc_array(char *, val_count);
>
> These two should not need casts.
>
> - stack = palloc(sizeof(*stack));
> + stack = palloc_object(sepgsql_proc_stack);
>
> This one in sepgsql was wrong, that can be detected with a compilation
> of the module.

My bad. I hadn't realized that - obviously - not necessarily all code is
actually compiled by default.

Will the build system enable any target for which all dependencies (e.g.
libraries) are met, or are there targets that need to be enabled
explicitly? Is there a way to enable all of them so I can easily make
sure, I actually compile all code?

> And done with the contrib/ part for the trival changes.

Thanks for applying the first batch of changes.

--
David Geier

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Pogrebnoi 2025-12-05 13:07:07 Popcount optimization for the slow-path lookups
Previous Message Amit Kapila 2025-12-05 12:10:28 Re: Newly created replication slot may be invalidated by checkpoint