Re: Coccinelle for PostgreSQL development [1/N]: coccicheck.py

On Sun, Jan 19, 2025 at 2:10 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Sat, Jan 18, 2025 at 08:44:00PM +0100, Mats Kindahl wrote:
> > For PostgreSQL 16, Peter extended the palloc()/pg_malloc() interface in
> > commit 2016055a92f to provide more type-safety, but these functions are
> not
> > widely used. This semantic patch captures and replaces all uses of
> palloc()
> > where palloc_array() or palloc_object() could be used instead. It
> > deliberately does not touch cases where it is not clear that the
> > replacement can be done.
>
> I am not sure how much a dependency to coccicheck would cost (usually
> such changes should require a case-by-case analysis rather than a
> blind automation), but palloc_array() and palloc_object() are
> available down to v13.

This script is intended to be conservative in that it should not do
replacements that are not clearly suitable for palloc_array/palloc_object.
Since the intention is that it should automatically generate patches on
cases that can be improved, I think the best strategy is to be conservative
and not do replacements unless it is clear that it is a good replacement.

> Based on this argument, it would be tempting to apply this rule
> across the stable branches to reduce conflict churn. However this is
> an improvement in readability, like the talloc() things as Peter has
> mentioned, hence it should be a HEAD-only thing.

I would argue that it is a HEAD-only thing. Main reason is that backports
always risk creating extra work, even if they look innocent, and it is
usually better to only backport patches that *really* need to be backported.

> I do like the idea
> of forcing more the object-palloc style on HEAD in the tree in some
> areas of the code, even if it would come with some backpatching cost
> for existing code.
>
> Thoughts? Perhaps this has been discussed previously?
>

My main reasoning around this patch is that the palloc_array and
palloc_object were introduced for a reason, in this case for type-safety
and readability, and not using it widely in the code base sort of defeats
the purpose of adding the functions at all. Doing it manually is a chore,
but with Coccinelle we can do these kinds of large rewrites easily.
--
Best wishes,
Mats Kindahl, Timescale