Re: Plug minor memleak in pg_dump

Em qui., 10 de fev. de 2022 às 10:57, Daniel Gustafsson <daniel(at)yesql(dot)se>
escreveu:

> > 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.
>
Well, the v2 aborts.

>
> > If ReadStr fails and returns NULL, sscanf will crash.
>
> Yes, which is better than silently propage the error.
>
Ok, silently propagating the error is bad, but crashing is a signal of
poor tool.

> > 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.
>
For me the code stays more simple and maintainable.

regards,
Ranier Vilela