Re: Expand palloc/pg_malloc API

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> On 17.05.22 20:43, Tom Lane wrote:
>> So I think Peter's got a good idea here (I might quibble with the details
>> of some of these macros). But it's not really going to move the
>> safety goalposts very far unless we make a concerted effort to make
>> these be the style everywhere. Are we willing to do that? What
>> will it do to back-patching difficulty? Dare I suggest back-patching
>> these changes?

> I think it could go like the castNode() introduction: first we adopt it
> sporadically for new code, then we change over some larger pieces of
> code, then we backpatch the API, then someone sends in a big patch to
> change the rest.

OK, that seems like a reasonable plan.

I've now studied this a little more closely, and I think the
functionality is fine, but I have naming quibbles.

1. Do we really want distinct names for the frontend and backend
versions of the macros? Particularly since you're layering the
frontend ones over pg_malloc, which has exit-on-OOM behavior?
I think we've found that notational discrepancies between frontend
and backend code are generally more a hindrance than a help,
so I'm inclined to drop the pg_malloc_xxx macros and just use
"palloc"-based names across the board.

2. I don't like the "palloc_ptrtype" name at all. I see that you
borrowed that name from talloc, but I doubt that's a precedent that
very many people are familiar with. To me it sounds like it might
allocate something that's the size of a pointer, not the size of the
pointed-to object. I have to confess though that I don't have an
obviously better name to suggest. "palloc_pointed_to" would be
clear perhaps, but it's kind of long.

3. Likewise, "palloc_obj" is perhaps less clear than it could be.
I find palloc_array just fine though. Maybe palloc_object or
palloc_struct? (If "_obj" can be traced to talloc, I'm not
seeing where.)

One thought that comes to mind is that palloc_ptrtype is almost
surely going to be used in the style

myvariable = palloc_ptrtype(myvariable);

and if it's not that it's very likely wrong. So maybe we should cut
out the middleman and write something like

#define palloc_instantiate(ptr) ((ptr) = (typeof(ptr)) palloc(sizeof(*(ptr))))
...
palloc_instantiate(myvariable);

I'm not wedded to "instantiate" here, there's probably better names.

regards, tom lane