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