Re: Coverity complains about simplehash.h's SH_STAT()

Hi,

On 2024-04-07 21:03:53 -0400, Tom Lane wrote:
> Today's Coverity run produced this:
>
> /srv/coverity/git/pgsql-git/postgresql/src/include/lib/simplehash.h: 1138 in bh_nodeidx_stat()
> 1132 avg_collisions = 0;
> 1133 }
> 1134
> 1135 sh_log("size: " UINT64_FORMAT ", members: %u, filled: %f, total chain: %u, max chain: %u, avg chain: %f, total_collisions: %u, max_collisions: %u, avg_collisions: %f",
> 1136 tb->size, tb->members, fillfactor, total_chain_length, max_chain_length, avg_chain_length,
> 1137 total_collisions, max_collisions, avg_collisions);
> >>> CID 1596268: Resource leaks (RESOURCE_LEAK)
> >>> Variable "collisions" going out of scope leaks the storage it points to.
> 1138 }
> 1139
> 1140 #endif /* SH_DEFINE */
>
> I have no idea why we didn't see this warning before --- but AFAICS
> it's quite right, and it looks like a nontrivial amount of memory
> could be at stake:
>
> uint32 *collisions = (uint32 *) palloc0(tb->size * sizeof(uint32));
>
> I realize this function is only debug support, but wouldn't it
> be appropriate to pfree(collisions) before exiting?

It indeed looks like that memory should be freed. Very odd that coverity
started to complain about that just now. If coverity had started to complain
after da41d71070d, I'd understand, but that was ~5 years ago.

I can't see a way it could hurt in the back branches, so I'm inclined to
backpatch the pfree?

Greetings,

Andres Freund