Fix and improve allocation formulas

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Fix and improve allocation formulas
Date: 2025-12-11 13:27:56
Message-ID: aTrG3Fi4APtfiCvQ@ip-10-97-1-34.eu-west-3.compute.internal
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi hackers,

Two allocation formulas have been fixed recently in 3f83de20ba2 and 06761b6096b,
so I looked for potential others with a coccinelle script [1].

It found two formulas that are technically correct, but using GBT_VARKEY and char
are the semantically appropriate choices (see 0001 attached).

Also, to make this safer, instead of:

"
var = palloc(sizeof(T) * count)
"

we could do:

"
var = palloc(sizeof(*var) * count)
"

that way the size computation is correct even if the variable's type changes (
less prone to errors and bugs then).

That would give something like in 0002 (produced with [2]).

Note that:

- 0002 is a very large patch. I think that it provides added value as mentioned
above but I'm not sure it is worth the noise. Anyway it is done, so sharing
here to get your thoughts.

- sizeof(*var) is evaluated at compile time so that's safe even with uninitialized pointers

- this is the preferred form for the Linux kernel (see "Allocating memory" in the
coding style doc [3])

- when there is casting involved, that might look weird to have the cast and not
computing the size on the "type". So, I've a mixed feeling about those even if I
think that's right to have a consistent approach.

Remarks:

- the patch does not touch the "test" files to reduce the noise
- we could do the same for:

"
var = palloc_array(T, count)
"

to

"
var = palloc_array(*var, count)
"

but that would not work because palloc_array is defined as:

#define palloc_array(type, count) ((type *) palloc(sizeof(type) * (count)))

and the cast would fail. We could use typeof() in palloc_array() but that leads
to the same discussion as in [4].

Thoughts?

[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/detect_sizeof_bugs.cocci
[2]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/use_var_in_sizeof.cocci
[3]: https://www.kernel.org/doc/html/latest/process/coding-style.html
[4]: https://www.postgresql.org/message-id/flat/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg%40mail.gmail.com

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v1-0001-Fix-memory-allocation-formulas.patch text/x-diff 1.5 KB
v1-0002-Use-sizeof-var-for-type-safe-allocation-formulas.patch text/x-diff 277.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Euler Taveira 2025-12-11 13:59:07 Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement
Previous Message Ashutosh Bapat 2025-12-11 13:17:21 Re: tablecmds: Open pg_class only when an update is required