| 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
| 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 |