On 26.07.22 23:32, Tom Lane wrote:
> 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.
This seems like a question that is independent of this patch. Given
that both pg_malloc() and palloc() do exist in fe_memutils, I think it
would be confusing to only extend one part of that and not the other.
The amount of code is ultimately not a lot.
If we wanted to get rid of pg_malloc() altogether, maybe we could talk
about that.
(Personally, I have always been a bit suspicious about using the name
palloc() without memory context semantics in frontend code, but I guess
this is wide-spread now.)
> 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.)
In talloc, the talloc() function itself allocates an object of a given
type. To allocate something of a specified size, you'd use
talloc_size(). So those names won't map exactly. I'm fine with
palloc_object() if that is clearer.
> 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);
Right, this is sort of what you'd want, really. But it looks like
strange C code, since you are modifying the variable even though you are
passing it by value.
I think the _ptrtype variant isn't that useful anyway, so if it's
confusing we can leave it out.
