Re: Plug minor memleak in pg_dump

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, gkokolatos(at)pm(dot)me
Subject: Re: Plug minor memleak in pg_dump
Date: 2022-02-10 13:57:24
Message-ID: 70019E5D-A6AB-43BA-84F9-D36EB8C678B6@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 10 Feb 2022, at 12:14, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> Em qua., 9 de fev. de 2022 às 23:16, Michael Paquier <michael(at)paquier(dot)xyz <mailto:michael(at)paquier(dot)xyz>> escreveu:

> This patch makes things worse, doesn't it?
> No.
>
> Doesn't this localized
> change mean that we expose ourselves more into *ignoring* TOC entries
> if we mess up with this code in the future?
> InvalidOid already used for "default".

There is no default case here, setting the tableoid to InvalidOid is done when
the archive doesn't support this particular feature. If we can't read the
tableoid here, it's a corrupt TOC and we should abort.

> If ReadStr fails and returns NULL, sscanf will crash.

Yes, which is better than silently propage the error.

> Maybe in this case, better report to the user?
> pg_log_warning?

That would demote what is today a crash to a warning on a corrupt TOC entry,
which I think is the wrong way to go. Question is, can this fail in a
non-synthetic case on output which was successfully generated by pg_dump? I'm
not saying we should ignore errors, but I have a feeling that any input fed
that triggers this will be broken enough to cause fireworks elsewhere too, and
this being a chase towards low returns apart from complicating the code.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeevan Ladhe 2022-02-10 14:31:50 Re: refactoring basebackup.c
Previous Message Ranier Vilela 2022-02-10 13:49:43 Re: Plug minor memleak in pg_dump