Re: Plug minor memleak in pg_dump

> On 2 Feb 2022, at 09:29, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Tue, 1 Feb 2022 19:48:01 +0530, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote in
>> On Tue, Feb 1, 2022 at 7:06 PM <gkokolatos(at)pm(dot)me> wrote:
>>>
>>> Hi,
>>>
>>> I noticed a minor memleak in pg_dump. ReadStr() returns a malloc'ed pointer which
>>> should then be freed. While reading the Table of Contents, it was called as an argument
>>> within a function call, leading to a memleak.
>>>
>>> Please accept the attached as a proposed fix.
>
> It is freed in other temporary use of the result of ReadStr(). So
> freeing it sounds sensible at a glance.
>
>> +1. IMO, having "restoring tables WITH OIDS is not supported anymore"
>> twice doesn't look good, how about as shown in [1]?
>
> Maybe [2] is smaller :)

It is smaller, but I think Bharath's version wins in terms of readability.

> Thus.. I came to doubt of its worthiness to the complexity. The
> amount of the leak is (perhaps) negligible.
>
> So, I would just write a comment there.

The leak itself is clearly not something to worry about wrt memory pressure.
We do read into tmp and free it in other places in the same function though (as
you note above), so for code consistency alone this is worth doing IMO (and it
reduces the risk of static analyzers flagging this).

Unless objected to I will go ahead with getting this committed.

--
Daniel Gustafsson https://vmware.com/