Skip to content

Commit 072e2f8

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 4907984 commit 072e2f8

File tree

1 file changed

+29
-45
lines changed

1 file changed

+29
-45
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ static char *convertRegProcReference(const char *proc);
271271
static char *getFormattedOperatorName(const char *oproid);
272272
static char *convertTSFunction(Archive *fout, Oid funcOid);
273273
static Oid findLastBuiltinOid_V71(Archive *fout);
274-
static char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
274+
static const char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
275275
static void getBlobs(Archive *fout);
276276
static void dumpBlob(Archive *fout, const BlobInfo *binfo);
277277
static int dumpBlobs(Archive *fout, const void *arg);
@@ -11252,13 +11252,9 @@ dumpBaseType(Archive *fout, const TypeInfo *tyinfo)
1125211252
appendPQExpBuffer(q, ",\n SUBSCRIPT = %s", typsubscript);
1125311253

1125411254
if (OidIsValid(tyinfo->typelem))
11255-
{
11256-
char *elemType;
11257-
11258-
elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroIsError);
11259-
appendPQExpBuffer(q, ",\n ELEMENT = %s", elemType);
11260-
free(elemType);
11261-
}
11255+
appendPQExpBuffer(q, ",\n ELEMENT = %s",
11256+
getFormattedTypeName(fout, tyinfo->typelem,
11257+
zeroIsError));
1126211258

1126311259
if (strcmp(typcategory, "U") != 0)
1126411260
{
@@ -12062,7 +12058,7 @@ format_function_arguments_old(Archive *fout,
1206212058
for (j = 0; j < nallargs; j++)
1206312059
{
1206412060
Oid typid;
12065-
char *typname;
12061+
const char *typname;
1206612062
const char *argmode;
1206712063
const char *argname;
1206812064

@@ -12101,7 +12097,6 @@ format_function_arguments_old(Archive *fout,
1210112097
argname ? fmtId(argname) : "",
1210212098
argname ? " " : "",
1210312099
typname);
12104-
free(typname);
1210512100
}
1210612101
appendPQExpBufferChar(&fn, ')');
1210712102
return fn.data;
@@ -12130,15 +12125,12 @@ format_function_signature(Archive *fout, const FuncInfo *finfo, bool honor_quote
1213012125
appendPQExpBuffer(&fn, "%s(", finfo->dobj.name);
1213112126
for (j = 0; j < finfo->nargs; j++)
1213212127
{
12133-
char *typname;
12134-
1213512128
if (j > 0)
1213612129
appendPQExpBufferStr(&fn, ", ");
1213712130

12138-
typname = getFormattedTypeName(fout, finfo->argtypes[j],
12139-
zeroIsError);
12140-
appendPQExpBufferStr(&fn, typname);
12141-
free(typname);
12131+
appendPQExpBufferStr(&fn,
12132+
getFormattedTypeName(fout, finfo->argtypes[j],
12133+
zeroIsError));
1214212134
}
1214312135
appendPQExpBufferChar(&fn, ')');
1214412136
return fn.data;
@@ -12183,7 +12175,6 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1218312175
char *prosupport;
1218412176
char *proparallel;
1218512177
char *lanname;
12186-
char *rettypename;
1218712178
int nallargs;
1218812179
char **allargtypes = NULL;
1218912180
char **argmodes = NULL;
@@ -12473,14 +12464,10 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1247312464
else if (funcresult)
1247412465
appendPQExpBuffer(q, " RETURNS %s", funcresult);
1247512466
else
12476-
{
12477-
rettypename = getFormattedTypeName(fout, finfo->prorettype,
12478-
zeroIsError);
1247912467
appendPQExpBuffer(q, " RETURNS %s%s",
1248012468
(proretset[0] == 't') ? "SETOF " : "",
12481-
rettypename);
12482-
free(rettypename);
12483-
}
12469+
getFormattedTypeName(fout, finfo->prorettype,
12470+
zeroIsError));
1248412471

1248512472
appendPQExpBuffer(q, "\n LANGUAGE %s", fmtId(lanname));
1248612473

@@ -12687,8 +12674,8 @@ dumpCast(Archive *fout, const CastInfo *cast)
1268712674
PQExpBuffer labelq;
1268812675
PQExpBuffer castargs;
1268912676
FuncInfo *funcInfo = NULL;
12690-
char *sourceType;
12691-
char *targetType;
12677+
const char *sourceType;
12678+
const char *targetType;
1269212679

1269312680
/* Skip if not to be dumped */
1269412681
if (!cast->dobj.dump || dopt->dataOnly)
@@ -12774,9 +12761,6 @@ dumpCast(Archive *fout, const CastInfo *cast)
1277412761
NULL, "",
1277512762
cast->dobj.catId, 0, cast->dobj.dumpId);
1277612763

12777-
free(sourceType);
12778-
free(targetType);
12779-
1278012764
destroyPQExpBuffer(defqry);
1278112765
destroyPQExpBuffer(delqry);
1278212766
destroyPQExpBuffer(labelq);
@@ -12797,7 +12781,7 @@ dumpTransform(Archive *fout, const TransformInfo *transform)
1279712781
FuncInfo *fromsqlFuncInfo = NULL;
1279812782
FuncInfo *tosqlFuncInfo = NULL;
1279912783
char *lanname;
12800-
char *transformType;
12784+
const char *transformType;
1280112785

1280212786
/* Skip if not to be dumped */
1280312787
if (!transform->dobj.dump || dopt->dataOnly)
@@ -12904,7 +12888,6 @@ dumpTransform(Archive *fout, const TransformInfo *transform)
1290412888
transform->dobj.catId, 0, transform->dobj.dumpId);
1290512889

1290612890
free(lanname);
12907-
free(transformType);
1290812891
destroyPQExpBuffer(defqry);
1290912892
destroyPQExpBuffer(delqry);
1291012893
destroyPQExpBuffer(labelq);
@@ -14202,17 +14185,11 @@ format_aggregate_signature(const AggInfo *agginfo, Archive *fout, bool honor_quo
1420214185
{
1420314186
appendPQExpBufferChar(&buf, '(');
1420414187
for (j = 0; j < agginfo->aggfn.nargs; j++)
14205-
{
14206-
char *typname;
14207-
14208-
typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j],
14209-
zeroIsError);
14210-
1421114188
appendPQExpBuffer(&buf, "%s%s",
1421214189
(j > 0) ? ", " : "",
14213-
typname);
14214-
free(typname);
14215-
}
14190+
getFormattedTypeName(fout,
14191+
agginfo->aggfn.argtypes[j],
14192+
zeroIsError));
1421614193
appendPQExpBufferChar(&buf, ')');
1421714194
}
1421814195
return buf.data;
@@ -18885,8 +18862,10 @@ findDumpableDependencies(ArchiveHandle *AH, const DumpableObject *dobj,
1888518862
*
1888618863
* This does not guarantee to schema-qualify the output, so it should not
1888718864
* be used to create the target object name for CREATE or ALTER commands.
18865+
*
18866+
* Note that the result is cached and must not be freed by the caller.
1888818867
*/
18889-
static char *
18868+
static const char *
1889018869
getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1889118870
{
1889218871
TypeInfo *typeInfo;
@@ -18897,15 +18876,15 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1889718876
if (oid == 0)
1889818877
{
1889918878
if ((opts & zeroAsStar) != 0)
18900-
return pg_strdup("*");
18879+
return "*";
1890118880
else if ((opts & zeroAsNone) != 0)
18902-
return pg_strdup("NONE");
18881+
return "NONE";
1890318882
}
1890418883

1890518884
/* see if we have the result cached in the type's TypeInfo record */
1890618885
typeInfo = findTypeByOid(oid);
1890718886
if (typeInfo && typeInfo->ftypname)
18908-
return pg_strdup(typeInfo->ftypname);
18887+
return typeInfo->ftypname;
1890918888

1891018889
query = createPQExpBuffer();
1891118890
appendPQExpBuffer(query, "SELECT pg_catalog.format_type('%u'::pg_catalog.oid, NULL)",
@@ -18919,9 +18898,14 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1891918898
PQclear(res);
1892018899
destroyPQExpBuffer(query);
1892118900

18922-
/* cache a copy for later requests */
18901+
/*
18902+
* Cache the result for re-use in later requests, if possible. If we
18903+
* don't have a TypeInfo for the type, the string will be leaked once the
18904+
* caller is done with it ... but that case really should not happen, so
18905+
* leaking if it does seems acceptable.
18906+
*/
1892318907
if (typeInfo)
18924-
typeInfo->ftypname = pg_strdup(result);
18908+
typeInfo->ftypname = result;
1892518909

1892618910
return result;
1892718911
}

0 commit comments

Comments
 (0)