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
