Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 70bef49

Browse files
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.
1 parent b3b4d8e commit 70bef49

File tree

3 files changed

+56
-51
lines changed

3 files changed

+56
-51
lines changed

src/bin/pg_dump/common.c

+20-22
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,8 @@ getSchemaData(Archive *fout, int *numTablesPtr)
260260
pg_log_info("reading subscriptions");
261261
getSubscriptions(fout);
262262

263+
free(inhinfo); /* not needed any longer */
264+
263265
*numTablesPtr = numTables;
264266
return tblinfo;
265267
}
@@ -373,24 +375,20 @@ static void
373375
flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
374376
{
375377
int i,
376-
j,
377-
k;
378+
j;
378379

379380
for (i = 0; i < numTables; i++)
380381
{
381-
IndexAttachInfo *attachinfo;
382-
383382
if (!tblinfo[i].ispartition || tblinfo[i].numParents == 0)
384383
continue;
385384

386385
Assert(tblinfo[i].numParents == 1);
387386

388-
attachinfo = (IndexAttachInfo *)
389-
pg_malloc0(tblinfo[i].numIndexes * sizeof(IndexAttachInfo));
390-
for (j = 0, k = 0; j < tblinfo[i].numIndexes; j++)
387+
for (j = 0; j < tblinfo[i].numIndexes; j++)
391388
{
392389
IndxInfo *index = &(tblinfo[i].indexes[j]);
393390
IndxInfo *parentidx;
391+
IndexAttachInfo *attachinfo;
394392

395393
if (index->parentidx == 0)
396394
continue;
@@ -399,14 +397,16 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
399397
if (parentidx == NULL)
400398
continue;
401399

402-
attachinfo[k].dobj.objType = DO_INDEX_ATTACH;
403-
attachinfo[k].dobj.catId.tableoid = 0;
404-
attachinfo[k].dobj.catId.oid = 0;
405-
AssignDumpId(&attachinfo[k].dobj);
406-
attachinfo[k].dobj.name = pg_strdup(index->dobj.name);
407-
attachinfo[k].dobj.namespace = index->indextable->dobj.namespace;
408-
attachinfo[k].parentIdx = parentidx;
409-
attachinfo[k].partitionIdx = index;
400+
attachinfo = (IndexAttachInfo *) pg_malloc(sizeof(IndexAttachInfo));
401+
402+
attachinfo->dobj.objType = DO_INDEX_ATTACH;
403+
attachinfo->dobj.catId.tableoid = 0;
404+
attachinfo->dobj.catId.oid = 0;
405+
AssignDumpId(&attachinfo->dobj);
406+
attachinfo->dobj.name = pg_strdup(index->dobj.name);
407+
attachinfo->dobj.namespace = index->indextable->dobj.namespace;
408+
attachinfo->parentIdx = parentidx;
409+
attachinfo->partitionIdx = index;
410410

411411
/*
412412
* We must state the DO_INDEX_ATTACH object's dependencies
@@ -423,17 +423,15 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
423423
* will not try to run the ATTACH concurrently with other
424424
* operations on those tables.
425425
*/
426-
addObjectDependency(&attachinfo[k].dobj, index->dobj.dumpId);
427-
addObjectDependency(&attachinfo[k].dobj, parentidx->dobj.dumpId);
428-
addObjectDependency(&attachinfo[k].dobj,
426+
addObjectDependency(&attachinfo->dobj, index->dobj.dumpId);
427+
addObjectDependency(&attachinfo->dobj, parentidx->dobj.dumpId);
428+
addObjectDependency(&attachinfo->dobj,
429429
index->indextable->dobj.dumpId);
430-
addObjectDependency(&attachinfo[k].dobj,
430+
addObjectDependency(&attachinfo->dobj,
431431
parentidx->indextable->dobj.dumpId);
432432

433433
/* keep track of the list of partitions in the parent index */
434-
simple_ptr_list_append(&parentidx->partattaches, &attachinfo[k].dobj);
435-
436-
k++;
434+
simple_ptr_list_append(&parentidx->partattaches, &attachinfo->dobj);
437435
}
438436
}
439437
}

src/bin/pg_dump/pg_backup_archiver.c

+2
Original file line numberDiff line numberDiff line change
@@ -3374,6 +3374,8 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
33743374

33753375
destroyPQExpBuffer(cmd);
33763376

3377+
if (AH->currTableAm)
3378+
free(AH->currTableAm);
33773379
AH->currTableAm = pg_strdup(want);
33783380
}
33793381

src/bin/pg_dump/pg_dump.c

+34-29
Original file line numberDiff line numberDiff line change
@@ -6869,7 +6869,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
68696869
PQExpBuffer query = createPQExpBuffer();
68706870
PGresult *res;
68716871
IndxInfo *indxinfo;
6872-
ConstraintInfo *constrinfo;
68736872
int i_tableoid,
68746873
i_oid,
68756874
i_indexname,
@@ -7133,7 +7132,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
71337132

71347133
tbinfo->indexes = indxinfo =
71357134
(IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo));
7136-
constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
71377135
tbinfo->numIndexes = ntups;
71387136

71397137
for (j = 0; j < ntups; j++)
@@ -7173,28 +7171,31 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
71737171
* If we found a constraint matching the index, create an
71747172
* entry for it.
71757173
*/
7176-
constrinfo[j].dobj.objType = DO_CONSTRAINT;
7177-
constrinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid));
7178-
constrinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid));
7179-
AssignDumpId(&constrinfo[j].dobj);
7180-
constrinfo[j].dobj.dump = tbinfo->dobj.dump;
7181-
constrinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_conname));
7182-
constrinfo[j].dobj.namespace = tbinfo->dobj.namespace;
7183-
constrinfo[j].contable = tbinfo;
7184-
constrinfo[j].condomain = NULL;
7185-
constrinfo[j].contype = contype;
7174+
ConstraintInfo *constrinfo;
7175+
7176+
constrinfo = (ConstraintInfo *) pg_malloc(sizeof(ConstraintInfo));
7177+
constrinfo->dobj.objType = DO_CONSTRAINT;
7178+
constrinfo->dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid));
7179+
constrinfo->dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid));
7180+
AssignDumpId(&constrinfo->dobj);
7181+
constrinfo->dobj.dump = tbinfo->dobj.dump;
7182+
constrinfo->dobj.name = pg_strdup(PQgetvalue(res, j, i_conname));
7183+
constrinfo->dobj.namespace = tbinfo->dobj.namespace;
7184+
constrinfo->contable = tbinfo;
7185+
constrinfo->condomain = NULL;
7186+
constrinfo->contype = contype;
71867187
if (contype == 'x')
7187-
constrinfo[j].condef = pg_strdup(PQgetvalue(res, j, i_condef));
7188+
constrinfo->condef = pg_strdup(PQgetvalue(res, j, i_condef));
71887189
else
7189-
constrinfo[j].condef = NULL;
7190-
constrinfo[j].confrelid = InvalidOid;
7191-
constrinfo[j].conindex = indxinfo[j].dobj.dumpId;
7192-
constrinfo[j].condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't';
7193-
constrinfo[j].condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't';
7194-
constrinfo[j].conislocal = true;
7195-
constrinfo[j].separate = true;
7196-
7197-
indxinfo[j].indexconstraint = constrinfo[j].dobj.dumpId;
7190+
constrinfo->condef = NULL;
7191+
constrinfo->confrelid = InvalidOid;
7192+
constrinfo->conindex = indxinfo[j].dobj.dumpId;
7193+
constrinfo->condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't';
7194+
constrinfo->condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't';
7195+
constrinfo->conislocal = true;
7196+
constrinfo->separate = true;
7197+
7198+
indxinfo[j].indexconstraint = constrinfo->dobj.dumpId;
71987199
}
71997200
else
72007201
{
@@ -11878,6 +11879,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1187811879
char *funcsig; /* identity signature */
1187911880
char *funcfullsig = NULL; /* full signature */
1188011881
char *funcsig_tag;
11882+
char *qual_funcsig;
1188111883
char *proretset;
1188211884
char *prosrc;
1188311885
char *probin;
@@ -12168,15 +12170,17 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1216812170

1216912171
funcsig_tag = format_function_signature(fout, finfo, false);
1217012172

12173+
qual_funcsig = psprintf("%s.%s",
12174+
fmtId(finfo->dobj.namespace->dobj.name),
12175+
funcsig);
12176+
1217112177
if (prokind[0] == PROKIND_PROCEDURE)
1217212178
keyword = "PROCEDURE";
1217312179
else
1217412180
keyword = "FUNCTION"; /* works for window functions too */
1217512181

12176-
appendPQExpBuffer(delqry, "DROP %s %s.%s;\n",
12177-
keyword,
12178-
fmtId(finfo->dobj.namespace->dobj.name),
12179-
funcsig);
12182+
appendPQExpBuffer(delqry, "DROP %s %s;\n",
12183+
keyword, qual_funcsig);
1218012184

1218112185
appendPQExpBuffer(q, "CREATE %s %s.%s",
1218212186
keyword,
@@ -12329,9 +12333,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1232912333

1233012334
append_depends_on_extension(fout, q, &finfo->dobj,
1233112335
"pg_catalog.pg_proc", keyword,
12332-
psprintf("%s.%s",
12333-
fmtId(finfo->dobj.namespace->dobj.name),
12334-
funcsig));
12336+
qual_funcsig);
1233512337

1233612338
if (dopt->binary_upgrade)
1233712339
binary_upgrade_extension_member(q, &finfo->dobj,
@@ -12376,6 +12378,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1237612378
if (funcfullsig)
1237712379
free(funcfullsig);
1237812380
free(funcsig_tag);
12381+
free(qual_funcsig);
1237912382
if (allargtypes)
1238012383
free(allargtypes);
1238112384
if (argmodes)
@@ -14768,6 +14771,8 @@ dumpForeignServer(Archive *fout, const ForeignServerInfo *srvinfo)
1476814771
srvinfo->rolname,
1476914772
srvinfo->dobj.catId, srvinfo->dobj.dumpId);
1477014773

14774+
PQclear(res);
14775+
1477114776
free(qsrvname);
1477214777

1477314778
destroyPQExpBuffer(q);

0 commit comments

Comments
 (0)