Skip to content

Commit b725666

Browse files
tglsfdcrobozmey
tglsfdc
authored and
robozmey
committed
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. (cherry-picked from 70bef49)
1 parent f2f132d commit b725666

3 files changed

Lines changed: 78 additions & 51 deletions

File tree

‎src/bin/pg_dump/common.c‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,8 @@ getSchemaData(Archive *fout, int *numTablesPtr)
280280
pg_log_info("reading subscriptions");
281281
getSubscriptions(fout);
282282

283+
free(inhinfo); /* not needed any longer */
284+
283285
*numTablesPtr = numTables;
284286
return tblinfo;
285287
}
@@ -401,24 +403,20 @@ static void
401403
flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
402404
{
403405
int i,
404-
j,
405-
k;
406+
j;
406407

407408
for (i = 0; i < numTables; i++)
408409
{
409-
IndexAttachInfo *attachinfo;
410-
411410
if (!tblinfo[i].ispartition || tblinfo[i].numParents == 0)
412411
continue;
413412

414413
Assert(tblinfo[i].numParents == 1);
415414

416-
attachinfo = (IndexAttachInfo *)
417-
pg_malloc0(tblinfo[i].numIndexes * sizeof(IndexAttachInfo));
418-
for (j = 0, k = 0; j < tblinfo[i].numIndexes; j++)
415+
for (j = 0; j < tblinfo[i].numIndexes; j++)
419416
{
420417
IndxInfo *index = &(tblinfo[i].indexes[j]);
421418
IndxInfo *parentidx;
419+
IndexAttachInfo *attachinfo;
422420

423421
if (index->parentidx == 0)
424422
continue;
@@ -427,14 +425,16 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
427425
if (parentidx == NULL)
428426
continue;
429427

430-
attachinfo[k].dobj.objType = DO_INDEX_ATTACH;
431-
attachinfo[k].dobj.catId.tableoid = 0;
432-
attachinfo[k].dobj.catId.oid = 0;
433-
AssignDumpId(&attachinfo[k].dobj);
434-
attachinfo[k].dobj.name = pg_strdup(index->dobj.name);
435-
attachinfo[k].dobj.namespace = index->indextable->dobj.namespace;
436-
attachinfo[k].parentIdx = parentidx;
437-
attachinfo[k].partitionIdx = index;
428+
attachinfo = (IndexAttachInfo *) pg_malloc(sizeof(IndexAttachInfo));
429+
430+
attachinfo->dobj.objType = DO_INDEX_ATTACH;
431+
attachinfo->dobj.catId.tableoid = 0;
432+
attachinfo->dobj.catId.oid = 0;
433+
AssignDumpId(&attachinfo->dobj);
434+
attachinfo->dobj.name = pg_strdup(index->dobj.name);
435+
attachinfo->dobj.namespace = index->indextable->dobj.namespace;
436+
attachinfo->parentIdx = parentidx;
437+
attachinfo->partitionIdx = index;
438438

439439
/*
440440
* We must state the DO_INDEX_ATTACH object's dependencies
@@ -451,17 +451,15 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
451451
* will not try to run the ATTACH concurrently with other
452452
* operations on those tables.
453453
*/
454-
addObjectDependency(&attachinfo[k].dobj, index->dobj.dumpId);
455-
addObjectDependency(&attachinfo[k].dobj, parentidx->dobj.dumpId);
456-
addObjectDependency(&attachinfo[k].dobj,
454+
addObjectDependency(&attachinfo->dobj, index->dobj.dumpId);
455+
addObjectDependency(&attachinfo->dobj, parentidx->dobj.dumpId);
456+
addObjectDependency(&attachinfo->dobj,
457457
index->indextable->dobj.dumpId);
458-
addObjectDependency(&attachinfo[k].dobj,
458+
addObjectDependency(&attachinfo->dobj,
459459
parentidx->indextable->dobj.dumpId);
460460

461461
/* keep track of the list of partitions in the parent index */
462-
simple_ptr_list_append(&parentidx->partattaches, &attachinfo[k].dobj);
463-
464-
k++;
462+
simple_ptr_list_append(&parentidx->partattaches, &attachinfo->dobj);
465463
}
466464
}
467465
}

‎src/bin/pg_dump/pg_backup_archiver.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3450,6 +3450,8 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
34503450

34513451
destroyPQExpBuffer(cmd);
34523452

3453+
if (AH->currTableAm)
3454+
free(AH->currTableAm);
34533455
AH->currTableAm = pg_strdup(want);
34543456
}
34553457

‎src/bin/pg_dump/pg_dump.c‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8255,7 +8255,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
82558255
PQExpBuffer query = createPQExpBuffer();
82568256
PGresult *res;
82578257
IndxInfo *indxinfo;
8258-
ConstraintInfo *constrinfo;
82598258
int i_tableoid,
82608259
i_oid,
82618260
i_indexname,
@@ -8486,7 +8485,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
84868485

84878486
tbinfo->indexes = indxinfo =
84888487
(IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo));
8489-
constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
84908488
tbinfo->numIndexes = ntups;
84918489

84928490
for (j = 0; j < ntups; j++)
@@ -8526,28 +8524,31 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
85268524
* If we found a constraint matching the index, create an
85278525
* entry for it.
85288526
*/
8529-
constrinfo[j].dobj.objType = DO_CONSTRAINT;
8530-
constrinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid));
8531-
constrinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid));
8532-
AssignDumpId(&constrinfo[j].dobj);
8533-
constrinfo[j].dobj.dump = tbinfo->dobj.dump;
8534-
constrinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_conname));
8535-
constrinfo[j].dobj.namespace = tbinfo->dobj.namespace;
8536-
constrinfo[j].contable = tbinfo;
8537-
constrinfo[j].condomain = NULL;
8538-
constrinfo[j].contype = contype;
8527+
ConstraintInfo *constrinfo;
8528+
8529+
constrinfo = (ConstraintInfo *) pg_malloc(sizeof(ConstraintInfo));
8530+
constrinfo->dobj.objType = DO_CONSTRAINT;
8531+
constrinfo->dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid));
8532+
constrinfo->dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid));
8533+
AssignDumpId(&constrinfo->dobj);
8534+
constrinfo->dobj.dump = tbinfo->dobj.dump;
8535+
constrinfo->dobj.name = pg_strdup(PQgetvalue(res, j, i_conname));
8536+
constrinfo->dobj.namespace = tbinfo->dobj.namespace;
8537+
constrinfo->contable = tbinfo;
8538+
constrinfo->condomain = NULL;
8539+
constrinfo->contype = contype;
85398540
if (contype == 'x')
8540-
constrinfo[j].condef = pg_strdup(PQgetvalue(res, j, i_condef));
8541+
constrinfo->condef = pg_strdup(PQgetvalue(res, j, i_condef));
85418542
else
8542-
constrinfo[j].condef = NULL;
8543-
constrinfo[j].confrelid = InvalidOid;
8544-
constrinfo[j].conindex = indxinfo[j].dobj.dumpId;
8545-
constrinfo[j].condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't';
8546-
constrinfo[j].condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't';
8547-
constrinfo[j].conislocal = true;
8548-
constrinfo[j].separate = true;
8549-
8550-
indxinfo[j].indexconstraint = constrinfo[j].dobj.dumpId;
8543+
constrinfo->condef = NULL;
8544+
constrinfo->confrelid = InvalidOid;
8545+
constrinfo->conindex = indxinfo[j].dobj.dumpId;
8546+
constrinfo->condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't';
8547+
constrinfo->condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't';
8548+
constrinfo->conislocal = true;
8549+
constrinfo->separate = true;
8550+
8551+
indxinfo[j].indexconstraint = constrinfo->dobj.dumpId;
85518552
}
85528553
else
85538554
{
@@ -13285,6 +13286,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1328513286
char *funcsig; /* identity signature */
1328613287
char *funcfullsig = NULL; /* full signature */
1328713288
char *funcsig_tag;
13289+
char *qual_funcsig;
1328813290
char *proretset;
1328913291
char *prosrc;
1329013292
char *probin;
@@ -13623,15 +13625,39 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1362313625

1362413626
funcsig_tag = format_function_signature(fout, finfo, false);
1362513627

13628+
if (prokind[0] == PROKIND_PROCEDURE)
13629+
keyword = "PROCEDURE";
13630+
else
13631+
{
13632+
configitems = NULL;
13633+
nconfigitems = 0;
13634+
}
13635+
13636+
if (funcargs)
13637+
{
13638+
/* 8.4 or later; we rely on server-side code for most of the work */
13639+
funcfullsig = format_function_arguments(finfo, funcargs, false);
13640+
funcsig = format_function_arguments(finfo, funciargs, false);
13641+
}
13642+
else
13643+
/* pre-8.4, do it ourselves */
13644+
funcsig = format_function_arguments_old(fout,
13645+
finfo, nallargs, allargtypes,
13646+
argmodes, argnames);
13647+
13648+
funcsig_tag = format_function_signature(fout, finfo, false);
13649+
13650+
qual_funcsig = psprintf("%s.%s",
13651+
fmtId(finfo->dobj.namespace->dobj.name),
13652+
funcsig);
13653+
1362613654
if (prokind[0] == PROKIND_PROCEDURE)
1362713655
keyword = "PROCEDURE";
1362813656
else
1362913657
keyword = "FUNCTION"; /* works for window functions too */
1363013658

13631-
appendPQExpBuffer(delqry, "DROP %s %s.%s;\n",
13632-
keyword,
13633-
fmtId(finfo->dobj.namespace->dobj.name),
13634-
funcsig);
13659+
appendPQExpBuffer(delqry, "DROP %s %s;\n",
13660+
keyword, qual_funcsig);
1363513661

1363613662
appendPQExpBuffer(q, "CREATE %s %s.%s",
1363713663
keyword,
@@ -13829,9 +13855,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1382913855

1383013856
append_depends_on_extension(fout, q, &finfo->dobj,
1383113857
"pg_catalog.pg_proc", keyword,
13832-
psprintf("%s.%s",
13833-
fmtId(finfo->dobj.namespace->dobj.name),
13834-
funcsig));
13858+
qual_funcsig);
1383513859

1383613860
if (dopt->binary_upgrade)
1383713861
binary_upgrade_extension_member(q, &finfo->dobj,
@@ -13876,6 +13900,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1387613900
if (funcfullsig)
1387713901
free(funcfullsig);
1387813902
free(funcsig_tag);
13903+
free(qual_funcsig);
1387913904
if (allargtypes)
1388013905
free(allargtypes);
1388113906
if (argmodes)
@@ -16499,6 +16524,8 @@ dumpForeignServer(Archive *fout, const ForeignServerInfo *srvinfo)
1649916524
srvinfo->rolname,
1650016525
srvinfo->dobj.catId, srvinfo->dobj.dumpId);
1650116526

16527+
PQclear(res);
16528+
1650216529
free(qsrvname);
1650316530

1650416531
destroyPQExpBuffer(q);

0 commit comments

Comments
 (0)