Message-ID: <adef550d-e776-4a91-be24-df7ca67fc3c1@gmail.com>
Date: Wed, 10 Dec 2025 12:48:35 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Consistently use palloc_object() and palloc_array()
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Thomas Munro <thomas.munro@gmail.com>,
 Chao Li <li.evan.chao@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <ED821EDD-6A1B-45B8-9005-31BCFFCB5A48@gmail.com>
 <a29b0cd9-576f-486f-ba45-5139ee33adcf@gmail.com>
 <524587.1764365323@sss.pgh.pa.us>
 <1e7f0c17-a965-4d5b-ad88-b435a4d97490@gmail.com>
 <CA+hUKGL+hiMRrAOqop1wmfpqTLg3GdyAJmO6KH6Dz3FjQvGN+w@mail.gmail.com>
 <143d3184-b575-49bd-8bc1-f37f45f010cc@gmail.com>
 <aS-F6LkeDBEly9nz@paquier.xyz> <534126.1764722446@sss.pgh.pa.us>
 <4ab4a12b-49e9-4ebf-9055-225c8055fed0@gmail.com>
 <aTKMtSMTh4eiSGMr@paquier.xyz> <aTikrhT9NvtKncUK@paquier.xyz>
Content-Language: en-US
From: David Geier <geidav.pg@gmail.com>
In-Reply-To: <aTikrhT9NvtKncUK@paquier.xyz>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/adef550d-e776-4a91-be24-df7ca67fc3c1%40gmail.com>
Precedence: bulk

Hi Michael!

Thanks for continuing applying the patches.

On 09.12.2025 23:37, Michael Paquier wrote:
> On Fri, Dec 05, 2025 at 04:41:41PM +0900, Michael Paquier wrote:
>> Thanks.  That's a lot to digest.
> 
> Digesting a bit more now..
> 
> -   char       *ret = palloc(sizeof(buf));
> +   char       *ret = palloc_array(char, sizeof(buf))
> 
> This one in dumputils.c is right, but I am not sure that it is an
> improvement compared to the statu-quo.

Apart from saving casts, using the _array() and _object() variants
improves readability as it's clear from the used macro what the code
intents to do.

For example. without knowing the type of buf, it's not immediately clear
if this is allocating an array of the size of buf, or if it's allocating
an object (e.g. if buf happened to be some struct that represents a
buffer object).

The amount of allocated memory would of course be the same, but how it's
going to be used can be nicely derived from the used macro.

> Here is a list of the files where I have noticed tha addition of
> casts, which are not required anymore:
> brin_tuple.c.
> gistbuildbuffers.c.
> genam.c.
> nbtdedup.c.
> nbtree.c.
> nbtsort.c.
> index.c
> execGrouping.c
> execMain.c
> relnode.c
> partbounds.c
> mcv.c
> spell.c
> arrayfuncs.c
> partcache.c
> 
> Perhaps you have used a semi-automatic process and missed these?

Do you mean in these files I forgot removing casts that got unnecessary
after using _array() / _object()? It's possible that I missed some,
given the large amount. Please fix them as you see fit.

> One can argue that this one in bernouilli.c is not really necessary,
> tsm_state is actually a void *.

As stated above: this change is not only about saving casts but the
macros convey the intent much better than a call to palloc().

> Among the 300-ish files changed in the backend, 48 of them had their
> builds slightly change.  The list of them is attached.

Do you mean the disassembly because the number of lines of code changes?

--
David Geier