On Wed, Feb 9, 2022 at 8:26 AM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Wed, Feb 02, 2022 at 10:06:13AM +0100, Daniel Gustafsson wrote:
> > 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.
>
> Looks like you forgot to apply that?
Attaching the patch that I suggested above, also the original patch
proposed by Georgios is at [1], leaving the decision to the committer
to pick up the best one.
[1]
https://www.postgresql.org/message-id/oZwKiUxFsVaetG2xOJp7Hwao8F1AKIdfFDQLNJrnwoaxmjyB-45r_aYmhgXHKLcMI3GT24m9L6HafSi2ns7WFxXe0mw2_tIJpD-Z3vb_eyI%3D%40pm.me
Regards,
Bharath Rupireddy.
From 335db3331a99a6cfc35608cdbec204d843b8ac55 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 9 Feb 2022 03:25:50 +0000
Subject: [PATCH v1] Fix a memory leak while reading Table of Contents
ReadStr() returns a malloc'ed pointer. Using it directly in a function
call results in a memleak. Rewrite to use a temporary buffer which is
then freed.
---
src/bin/pg_dump/pg_backup_archiver.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 49bf0907cd..e62be78982 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2500,6 +2500,8 @@ ReadToc(ArchiveHandle *AH)
for (i = 0; i < AH->tocCount; i++)
{
+ bool is_supported = true;
+
te = (TocEntry *) pg_malloc0(sizeof(TocEntry));
te->dumpId = ReadInt(AH);
@@ -2574,7 +2576,20 @@ ReadToc(ArchiveHandle *AH)
te->tableam = ReadStr(AH);
te->owner = ReadStr(AH);
- if (AH->version < K_VERS_1_9 || strcmp(ReadStr(AH), "true") == 0)
+
+ if (AH->version < K_VERS_1_9)
+ is_supported = false;
+ else
+ {
+ tmp = ReadStr(AH);
+
+ if (strcmp(tmp, "true") == 0)
+ is_supported = false;
+
+ pg_free(tmp);
+ }
+
+ if (!is_supported)
pg_log_warning("restoring tables WITH OIDS is not supported anymore");
/* Read TOC entry dependencies */
--
2.25.1
