On Wed, Jun 05, 2024 at 01:58:54PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> On Wed, Jun 05, 2024 at 12:22:03PM -0400, Neil Conway wrote:
>>> (2) These functions malloc() a single ntups * sizeof(struct) allocation and
>>> then index into it to fill-in each struct before entering it into the hash
>>> table. It might be more straightforward to just malloc each individual
>>> struct.
>
>> That'd increase the number of allocations quite significantly, but I'd be
>> surprised if that was noticeable outside of extreme scenarios. At the
>> moment, I'm inclined to leave these as-is for this reason and because I
>> doubt it'd result in much cleanup, but I'll yield to the majority opinion
>> here.
>
> I think that would be quite an invasive change; it would require
> many hundreds of edits like
>
> - finfo[i].dobj.objType = DO_FUNC;
> + finfo->dobj.objType = DO_FUNC;
>
> which aside from being tedious would create a back-patching hazard.
> So I'm kind of -0.1 or so.
>
> Another angle to this is that Coverity and possibly other tools tend
> to report that these functions leak these allocations, apparently
> because they don't notice that pointers into the allocations get
> stored in hash tables by a subroutine. I'm not sure if making this
> change would make that worse or better. If we really want to change
> it, that might be worth checking somehow before we jump.
At the moment, I'm inclined to commit v1 once v18 development opens up. We
can consider any additional adjustments separately.
--
nathan