Re: Plug minor memleak in pg_dump - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Plug minor memleak in pg_dump
Date
Msg-id CAEudQApqFTsmeYBJbnA7ANGDm1Hd7Vu8QEGjsBya31vKawrrYQ@mail.gmail.com
Whole thread Raw
In response to Re: Plug minor memleak in pg_dump  (Ranier Vilela <ranier.vf@gmail.com>)
List pgsql-hackers
Em qui., 10 de fev. de 2022 às 08:14, Ranier Vilela <ranier.vf@gmail.com> escreveu:
Em qua., 9 de fev. de 2022 às 23:16, Michael Paquier <michael@paquier.xyz> escreveu:
On Wed, Feb 09, 2022 at 02:48:35PM -0300, Ranier Vilela wrote:
> IMO I think that still have troubles here.
>
> ReadStr can return NULL, so the fix can crash.

-           sscanf(tmp, "%u", &te->catalogId.tableoid);
-           free(tmp);
+           if (tmp)
+           {
+               sscanf(tmp, "%u", &te->catalogId.tableoid);
+               free(tmp);
+           }
+           else
+               te->catalogId.tableoid = InvalidOid;

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".
If ReadStr fails and returns NULL, sscanf will crash.

Maybe in this case, better report to the user?
pg_log_warning?
Maybe in this case, the right thing is abort?
 
See v2, please.

regards,
Ranier Vilela
Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: [BUG]Update Toast data failure in logical replication
Next
From: Daniel Gustafsson
Date:
Subject: Re: Plug minor memleak in pg_dump