On Wed, Jun 05, 2024 at 01:58:54PM -0400, Tom Lane wrote: > Nathan Bossart <nathandboss...@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
