Re: small pg_dump code cleanup

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