From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Geier <geidav.pg@gmail.com>
cc: Chao Li <li.evan.chao@gmail.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Consistently use palloc_object() and palloc_array()
In-reply-to: <a29b0cd9-576f-486f-ba45-5139ee33adcf@gmail.com>
References: <ad0748d4-3080-436e-b0bc-ac8f86a3466a@gmail.com>
 <ED821EDD-6A1B-45B8-9005-31BCFFCB5A48@gmail.com>
 <a29b0cd9-576f-486f-ba45-5139ee33adcf@gmail.com>
Comments: In-reply-to David Geier <geidav.pg@gmail.com>
	message dated "Fri, 28 Nov 2025 22:06:34 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <524586.1764365323.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 28 Nov 2025 16:28:43 -0500
Message-ID: <524587.1764365323@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/524587.1764365323%40sss.pgh.pa.us>
Precedence: bulk

David Geier <geidav.pg@gmail.com> writes:
> On 27.11.2025 00:03, Chao Li wrote:
>> This is a large patch, I just take a quick look, and found that:
>> -		*phoned_word =3D palloc(sizeof(char) * strlen(word) + 1);
>> +		*phoned_word =3D palloc_array(char, strlen(word) + 1);
>> And
>> -		params =3D (const char **) palloc(sizeof(char *));
>> +		params =3D palloc_object(const char *);
>> Applying palloc_array and palloc_object to char type doesn=E2=80=99t se=
em to improve anything.

> You mean because sizeof(char) is always 1 and hence we could instead
> simply write:
> 		*phoned_word =3D palloc(strlen(word) + 1);
> 		params =3D palloc(1);
> I think the _array and _object variants are more expressive and for sure
> don't make the code less readable.

Yeah, I agree these particular changes seem fine.  When you're doing
address arithmetic for a memcpy or such, it may be fine to wire in an
assumption that sizeof(char) =3D=3D 1, but I think doing that in other
contexts is not particularly good style.

Another thing to note is that the proposed patch effectively changes
the expression evaluation order:

-		*phoned_word =3D palloc((sizeof(char) * strlen(word)) + 1);
+		*phoned_word =3D palloc(sizeof(char) * (strlen(word) + 1));

Now, there's not actually any difference because sizeof(char) is 1,
but if it hypothetically weren't, the new version is likely more
correct.  Presumably the +1 is meant to allow room for a trailing \0,
which is a char.

It'd be a good idea to review the patch to see if there are any
places where semantics are changed in a less benign fashion...

			regards, tom lane