Fix minor memory leaks in pg_dump.

I found these by running pg_dump under "valgrind --leak-check=full".

The changes in flagInhIndexes() and getIndexes() replace allocation of
an array of which we use only some elements by individual allocations
of just the actually-needed objects.  The previous coding wasted some
memory, but more importantly it confused valgrind's leak tracking.

collectComments() and collectSecLabels() remain major blots on
the valgrind report, because they don't PQclear their query
results, in order to avoid a lot of strdup's.  That's a dubious
tradeoff, but I'll leave it alone here; an upcoming patch will
modify those functions enough to justify changing the tradeoff.

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 0170fe036fa691d62b25dd135e3eca96de0f0694..e38ff3c030a6ed6908c4acbe42d7b509156bad39 100644 (file)
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -260,6 +260,8 @@ getSchemaData(Archive *fout, int *numTablesPtr)
        pg_log_info("reading subscriptions");
        getSubscriptions(fout);
 
+       free(inhinfo);                          /* not needed any longer */
+
        *numTablesPtr = numTables;
        return tblinfo;
 }
@@ -373,24 +375,20 @@ static void
 flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 {
        int                     i,
-                               j,
-                               k;
+                               j;
 
        for (i = 0; i < numTables; i++)
        {
-               IndexAttachInfo *attachinfo;
-
                if (!tblinfo[i].ispartition || tblinfo[i].numParents == 0)
                        continue;
 
                Assert(tblinfo[i].numParents == 1);
 
-               attachinfo = (IndexAttachInfo *)
-                       pg_malloc0(tblinfo[i].numIndexes * sizeof(IndexAttachInfo));
-               for (j = 0, k = 0; j < tblinfo[i].numIndexes; j++)
+               for (j = 0; j < tblinfo[i].numIndexes; j++)
                {
                        IndxInfo   *index = &(tblinfo[i].indexes[j]);
                        IndxInfo   *parentidx;
+                       IndexAttachInfo *attachinfo;
 
                        if (index->parentidx == 0)
                                continue;
@@ -399,14 +397,16 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
                        if (parentidx == NULL)
                                continue;
 
-                       attachinfo[k].dobj.objType = DO_INDEX_ATTACH;
-                       attachinfo[k].dobj.catId.tableoid = 0;
-                       attachinfo[k].dobj.catId.oid = 0;
-                       AssignDumpId(&attachinfo[k].dobj);
-                       attachinfo[k].dobj.name = pg_strdup(index->dobj.name);
-                       attachinfo[k].dobj.namespace = index->indextable->dobj.namespace;
-                       attachinfo[k].parentIdx = parentidx;
-                       attachinfo[k].partitionIdx = index;
+                       attachinfo = (IndexAttachInfo *) pg_malloc(sizeof(IndexAttachInfo));
+
+                       attachinfo->dobj.objType = DO_INDEX_ATTACH;
+                       attachinfo->dobj.catId.tableoid = 0;
+                       attachinfo->dobj.catId.oid = 0;
+                       AssignDumpId(&attachinfo->dobj);
+                       attachinfo->dobj.name = pg_strdup(index->dobj.name);
+                       attachinfo->dobj.namespace = index->indextable->dobj.namespace;
+                       attachinfo->parentIdx = parentidx;
+                       attachinfo->partitionIdx = index;
 
                        /*
                         * We must state the DO_INDEX_ATTACH object's dependencies
@@ -423,17 +423,15 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
                         * will not try to run the ATTACH concurrently with other
                         * operations on those tables.
                         */
-                       addObjectDependency(&attachinfo[k].dobj, index->dobj.dumpId);
-                       addObjectDependency(&attachinfo[k].dobj, parentidx->dobj.dumpId);
-                       addObjectDependency(&attachinfo[k].dobj,
+                       addObjectDependency(&attachinfo->dobj, index->dobj.dumpId);
+                       addObjectDependency(&attachinfo->dobj, parentidx->dobj.dumpId);
+                       addObjectDependency(&attachinfo->dobj,
                                                                index->indextable->dobj.dumpId);
-                       addObjectDependency(&attachinfo[k].dobj,
+                       addObjectDependency(&attachinfo->dobj,
                                                                parentidx->indextable->dobj.dumpId);
 
                        /* keep track of the list of partitions in the parent index */
-                       simple_ptr_list_append(&parentidx->partattaches, &attachinfo[k].dobj);
-
-                       k++;
+                       simple_ptr_list_append(&parentidx->partattaches, &attachinfo->dobj);
                }
        }
 }

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index ee06dc68221eff65439faae76dbe4acb3dae33f7..9b0e699ce8621123c19ffdf6436ff57fd3060b64 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3374,6 +3374,8 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
 
        destroyPQExpBuffer(cmd);
 
+       if (AH->currTableAm)
+               free(AH->currTableAm);
        AH->currTableAm = pg_strdup(want);
 }
 

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b401b923ac759670360cbb090fb8d417a5dc0a62..e9f68e8986719296892738dc7cffb6f8ca7ae2c8 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6869,7 +6869,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
        PQExpBuffer query = createPQExpBuffer();
        PGresult   *res;
        IndxInfo   *indxinfo;
-       ConstraintInfo *constrinfo;
        int                     i_tableoid,
                                i_oid,
                                i_indexname,
@@ -7133,7 +7132,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 
                tbinfo->indexes = indxinfo =
                        (IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo));
-               constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
                tbinfo->numIndexes = ntups;
 
                for (j = 0; j < ntups; j++)
@@ -7173,28 +7171,31 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
                                 * If we found a constraint matching the index, create an
                                 * entry for it.
                                 */
-                               constrinfo[j].dobj.objType = DO_CONSTRAINT;
-                               constrinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid));
-                               constrinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid));
-                               AssignDumpId(&constrinfo[j].dobj);
-                               constrinfo[j].dobj.dump = tbinfo->dobj.dump;
-                               constrinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_conname));
-                               constrinfo[j].dobj.namespace = tbinfo->dobj.namespace;
-                               constrinfo[j].contable = tbinfo;
-                               constrinfo[j].condomain = NULL;
-                               constrinfo[j].contype = contype;
+                               ConstraintInfo *constrinfo;
+
+                               constrinfo = (ConstraintInfo *) pg_malloc(sizeof(ConstraintInfo));
+                               constrinfo->dobj.objType = DO_CONSTRAINT;
+                               constrinfo->dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid));
+                               constrinfo->dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid));
+                               AssignDumpId(&constrinfo->dobj);
+                               constrinfo->dobj.dump = tbinfo->dobj.dump;
+                               constrinfo->dobj.name = pg_strdup(PQgetvalue(res, j, i_conname));
+                               constrinfo->dobj.namespace = tbinfo->dobj.namespace;
+                               constrinfo->contable = tbinfo;
+                               constrinfo->condomain = NULL;
+                               constrinfo->contype = contype;
                                if (contype == 'x')
-                                       constrinfo[j].condef = pg_strdup(PQgetvalue(res, j, i_condef));
+                                       constrinfo->condef = pg_strdup(PQgetvalue(res, j, i_condef));
                                else
-                                       constrinfo[j].condef = NULL;
-                               constrinfo[j].confrelid = InvalidOid;
-                               constrinfo[j].conindex = indxinfo[j].dobj.dumpId;
-                               constrinfo[j].condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't';
-                               constrinfo[j].condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't';
-                               constrinfo[j].conislocal = true;
-                               constrinfo[j].separate = true;
-
-                               indxinfo[j].indexconstraint = constrinfo[j].dobj.dumpId;
+                                       constrinfo->condef = NULL;
+                               constrinfo->confrelid = InvalidOid;
+                               constrinfo->conindex = indxinfo[j].dobj.dumpId;
+                               constrinfo->condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't';
+                               constrinfo->condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't';
+                               constrinfo->conislocal = true;
+                               constrinfo->separate = true;
+
+                               indxinfo[j].indexconstraint = constrinfo->dobj.dumpId;
                        }
                        else
                        {
@@ -11878,6 +11879,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
        char       *funcsig;            /* identity signature */
        char       *funcfullsig = NULL; /* full signature */
        char       *funcsig_tag;
+       char       *qual_funcsig;
        char       *proretset;
        char       *prosrc;
        char       *probin;
@@ -12168,15 +12170,17 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
 
        funcsig_tag = format_function_signature(fout, finfo, false);
 
+       qual_funcsig = psprintf("%s.%s",
+                                                       fmtId(finfo->dobj.namespace->dobj.name),
+                                                       funcsig);
+
        if (prokind[0] == PROKIND_PROCEDURE)
                keyword = "PROCEDURE";
        else
                keyword = "FUNCTION";   /* works for window functions too */
 
-       appendPQExpBuffer(delqry, "DROP %s %s.%s;\n",
-                                         keyword,
-                                         fmtId(finfo->dobj.namespace->dobj.name),
-                                         funcsig);
+       appendPQExpBuffer(delqry, "DROP %s %s;\n",
+                                         keyword, qual_funcsig);
 
        appendPQExpBuffer(q, "CREATE %s %s.%s",
                                          keyword,
@@ -12329,9 +12333,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
 
        append_depends_on_extension(fout, q, &finfo->dobj,
                                                                "pg_catalog.pg_proc", keyword,
-                                                               psprintf("%s.%s",
-                                                                                fmtId(finfo->dobj.namespace->dobj.name),
-                                                                                funcsig));
+                                                               qual_funcsig);
 
        if (dopt->binary_upgrade)
                binary_upgrade_extension_member(q, &finfo->dobj,
@@ -12376,6 +12378,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
        if (funcfullsig)
                free(funcfullsig);
        free(funcsig_tag);
+       free(qual_funcsig);
        if (allargtypes)
                free(allargtypes);
        if (argmodes)
@@ -14768,6 +14771,8 @@ dumpForeignServer(Archive *fout, const ForeignServerInfo *srvinfo)
                                                 srvinfo->rolname,
                                                 srvinfo->dobj.catId, srvinfo->dobj.dumpId);
 
+       PQclear(res);
+
        free(qsrvname);
 
        destroyPQExpBuffer(q);