Re: Plug minor memleak in pg_dump

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.

+1. IMO, having "restoring tables WITH OIDS is not supported anymore"
twice doesn't look good, how about as shown in [1]?

[1]
diff --git a/src/bin/pg_dump/pg_backup_archiver.c
b/src/bin/pg_dump/pg_backup_archiver.c
index 49bf0907cd..777ff6fcfe 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2494,6 +2494,7 @@ ReadToc(ArchiveHandle *AH)
int depIdx;
int depSize;
TocEntry *te;
+ bool is_supported = true;

AH->tocCount = ReadInt(AH);
AH->maxDumpId = 0;
@@ -2574,7 +2575,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 */

Regards,
Bharath Rupireddy.