Skip to content

Commit df290e5

Browse files
tglsfdctglsfdc
committed
Avoid useless malloc/free traffic around getFormattedTypeName().
Coverity complained that one caller of getFormattedTypeName() failed to free the returned string. Which is true, but rather than fixing that one, let's get rid of this tedious and error-prone requirement. Now that getFormattedTypeName() caches its result, strdup'ing that result and expecting the caller to free it accomplishes little except to waste cycles. We do create a leak in the case where getTypes didn't make a TypeInfo for the type, but that basically shouldn't ever happen. Back-patch, as commit 6c450a8 was. This isn't a particularly interesting bug fix, but the API change seems like a hazard for future back-patching activity if we don't back-patch it.
1 parent 1fedbcc commit df290e5

File tree

1 file changed

+31
-47
lines changed

1 file changed

+31
-47
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 31 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ static char *convertRegProcReference(Archive *fout,
261261
static char *getFormattedOperatorName(Archive *fout, const char *oproid);
262262
static char *convertTSFunction(Archive *fout, Oid funcOid);
263263
static Oid findLastBuiltinOid_V71(Archive *fout);
264-
static char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
264+
static const char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
265265
static void getBlobs(Archive *fout);
266266
static void dumpBlob(Archive *fout, BlobInfo *binfo);
267267
static int dumpBlobs(Archive *fout, void *arg);
@@ -10889,13 +10889,9 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
1088910889
}
1089010890

1089110891
if (OidIsValid(tyinfo->typelem))
10892-
{
10893-
char *elemType;
10894-
10895-
elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroAsOpaque);
10896-
appendPQExpBuffer(q, ",\n ELEMENT = %s", elemType);
10897-
free(elemType);
10898-
}
10892+
appendPQExpBuffer(q, ",\n ELEMENT = %s",
10893+
getFormattedTypeName(fout, tyinfo->typelem,
10894+
zeroAsOpaque));
1089910895

1090010896
if (strcmp(typcategory, "U") != 0)
1090110897
{
@@ -11698,7 +11694,7 @@ format_function_arguments_old(Archive *fout,
1169811694
for (j = 0; j < nallargs; j++)
1169911695
{
1170011696
Oid typid;
11701-
char *typname;
11697+
const char *typname;
1170211698
const char *argmode;
1170311699
const char *argname;
1170411700

@@ -11737,7 +11733,6 @@ format_function_arguments_old(Archive *fout,
1173711733
argname ? fmtId(argname) : "",
1173811734
argname ? " " : "",
1173911735
typname);
11740-
free(typname);
1174111736
}
1174211737
appendPQExpBufferChar(&fn, ')');
1174311738
return fn.data;
@@ -11766,15 +11761,12 @@ format_function_signature(Archive *fout, FuncInfo *finfo, bool honor_quotes)
1176611761
appendPQExpBuffer(&fn, "%s(", finfo->dobj.name);
1176711762
for (j = 0; j < finfo->nargs; j++)
1176811763
{
11769-
char *typname;
11770-
1177111764
if (j > 0)
1177211765
appendPQExpBufferStr(&fn, ", ");
1177311766

11774-
typname = getFormattedTypeName(fout, finfo->argtypes[j],
11775-
zeroAsOpaque);
11776-
appendPQExpBufferStr(&fn, typname);
11777-
free(typname);
11767+
appendPQExpBufferStr(&fn,
11768+
getFormattedTypeName(fout, finfo->argtypes[j],
11769+
zeroAsOpaque));
1177811770
}
1177911771
appendPQExpBufferChar(&fn, ')');
1178011772
return fn.data;
@@ -11818,7 +11810,6 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1181811810
char *prosupport;
1181911811
char *proparallel;
1182011812
char *lanname;
11821-
char *rettypename;
1182211813
int nallargs;
1182311814
char **allargtypes = NULL;
1182411815
char **argmodes = NULL;
@@ -12175,14 +12166,10 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1217512166
else if (funcresult)
1217612167
appendPQExpBuffer(q, " RETURNS %s", funcresult);
1217712168
else
12178-
{
12179-
rettypename = getFormattedTypeName(fout, finfo->prorettype,
12180-
zeroAsOpaque);
1218112169
appendPQExpBuffer(q, " RETURNS %s%s",
1218212170
(proretset[0] == 't') ? "SETOF " : "",
12183-
rettypename);
12184-
free(rettypename);
12185-
}
12171+
getFormattedTypeName(fout, finfo->prorettype,
12172+
zeroAsOpaque));
1218612173

1218712174
appendPQExpBuffer(q, "\n LANGUAGE %s", fmtId(lanname));
1218812175

@@ -12389,8 +12376,8 @@ dumpCast(Archive *fout, CastInfo *cast)
1238912376
PQExpBuffer labelq;
1239012377
PQExpBuffer castargs;
1239112378
FuncInfo *funcInfo = NULL;
12392-
char *sourceType;
12393-
char *targetType;
12379+
const char *sourceType;
12380+
const char *targetType;
1239412381

1239512382
/* Skip if not to be dumped */
1239612383
if (!cast->dobj.dump || dopt->dataOnly)
@@ -12476,9 +12463,6 @@ dumpCast(Archive *fout, CastInfo *cast)
1247612463
NULL, "",
1247712464
cast->dobj.catId, 0, cast->dobj.dumpId);
1247812465

12479-
free(sourceType);
12480-
free(targetType);
12481-
1248212466
destroyPQExpBuffer(defqry);
1248312467
destroyPQExpBuffer(delqry);
1248412468
destroyPQExpBuffer(labelq);
@@ -12499,7 +12483,7 @@ dumpTransform(Archive *fout, TransformInfo *transform)
1249912483
FuncInfo *fromsqlFuncInfo = NULL;
1250012484
FuncInfo *tosqlFuncInfo = NULL;
1250112485
char *lanname;
12502-
char *transformType;
12486+
const char *transformType;
1250312487

1250412488
/* Skip if not to be dumped */
1250512489
if (!transform->dobj.dump || dopt->dataOnly)
@@ -12606,7 +12590,6 @@ dumpTransform(Archive *fout, TransformInfo *transform)
1260612590
transform->dobj.catId, 0, transform->dobj.dumpId);
1260712591

1260812592
free(lanname);
12609-
free(transformType);
1261012593
destroyPQExpBuffer(defqry);
1261112594
destroyPQExpBuffer(delqry);
1261212595
destroyPQExpBuffer(labelq);
@@ -13898,17 +13881,11 @@ format_aggregate_signature(AggInfo *agginfo, Archive *fout, bool honor_quotes)
1389813881
{
1389913882
appendPQExpBufferChar(&buf, '(');
1390013883
for (j = 0; j < agginfo->aggfn.nargs; j++)
13901-
{
13902-
char *typname;
13903-
13904-
typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j],
13905-
zeroAsOpaque);
13906-
1390713884
appendPQExpBuffer(&buf, "%s%s",
1390813885
(j > 0) ? ", " : "",
13909-
typname);
13910-
free(typname);
13911-
}
13886+
getFormattedTypeName(fout,
13887+
agginfo->aggfn.argtypes[j],
13888+
zeroAsOpaque));
1391213889
appendPQExpBufferChar(&buf, ')');
1391313890
}
1391413891
return buf.data;
@@ -18604,8 +18581,10 @@ findDumpableDependencies(ArchiveHandle *AH, DumpableObject *dobj,
1860418581
*
1860518582
* This does not guarantee to schema-qualify the output, so it should not
1860618583
* be used to create the target object name for CREATE or ALTER commands.
18584+
*
18585+
* Note that the result is cached and must not be freed by the caller.
1860718586
*/
18608-
static char *
18587+
static const char *
1860918588
getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1861018589
{
1861118590
TypeInfo *typeInfo;
@@ -18616,19 +18595,19 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1861618595
if (oid == 0)
1861718596
{
1861818597
if ((opts & zeroAsOpaque) != 0)
18619-
return pg_strdup(g_opaque_type);
18598+
return g_opaque_type;
1862018599
else if ((opts & zeroAsAny) != 0)
18621-
return pg_strdup("'any'");
18600+
return "'any'";
1862218601
else if ((opts & zeroAsStar) != 0)
18623-
return pg_strdup("*");
18602+
return "*";
1862418603
else if ((opts & zeroAsNone) != 0)
18625-
return pg_strdup("NONE");
18604+
return "NONE";
1862618605
}
1862718606

1862818607
/* see if we have the result cached in the type's TypeInfo record */
1862918608
typeInfo = findTypeByOid(oid);
1863018609
if (typeInfo && typeInfo->ftypname)
18631-
return pg_strdup(typeInfo->ftypname);
18610+
return typeInfo->ftypname;
1863218611

1863318612
query = createPQExpBuffer();
1863418613
appendPQExpBuffer(query, "SELECT pg_catalog.format_type('%u'::pg_catalog.oid, NULL)",
@@ -18642,9 +18621,14 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1864218621
PQclear(res);
1864318622
destroyPQExpBuffer(query);
1864418623

18645-
/* cache a copy for later requests */
18624+
/*
18625+
* Cache the result for re-use in later requests, if possible. If we
18626+
* don't have a TypeInfo for the type, the string will be leaked once the
18627+
* caller is done with it ... but that case really should not happen, so
18628+
* leaking if it does seems acceptable.
18629+
*/
1864618630
if (typeInfo)
18647-
typeInfo->ftypname = pg_strdup(result);
18631+
typeInfo->ftypname = result;
1864818632

1864918633
return result;
1865018634
}

0 commit comments

Comments
 (0)