Skip to content

Commit 52c300d

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 67948a4 commit 52c300d

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
@@ -262,7 +262,7 @@ static char *convertRegProcReference(const char *proc);
262262
static char *getFormattedOperatorName(const char *oproid);
263263
static char *convertTSFunction(Archive *fout, Oid funcOid);
264264
static Oid findLastBuiltinOid_V71(Archive *fout);
265-
static char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
265+
static const char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
266266
static void getBlobs(Archive *fout);
267267
static void dumpBlob(Archive *fout, const BlobInfo *binfo);
268268
static int dumpBlobs(Archive *fout, const void *arg);
@@ -11121,13 +11121,9 @@ dumpBaseType(Archive *fout, const TypeInfo *tyinfo)
1112111121
appendPQExpBuffer(q, ",\n SUBSCRIPT = %s", typsubscript);
1112211122

1112311123
if (OidIsValid(tyinfo->typelem))
11124-
{
11125-
char *elemType;
11126-
11127-
elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroIsError);
11128-
appendPQExpBuffer(q, ",\n ELEMENT = %s", elemType);
11129-
free(elemType);
11130-
}
11124+
appendPQExpBuffer(q, ",\n ELEMENT = %s",
11125+
getFormattedTypeName(fout, tyinfo->typelem,
11126+
zeroIsError));
1113111127

1113211128
if (strcmp(typcategory, "U") != 0)
1113311129
{
@@ -11931,7 +11927,7 @@ format_function_arguments_old(Archive *fout,
1193111927
for (j = 0; j < nallargs; j++)
1193211928
{
1193311929
Oid typid;
11934-
char *typname;
11930+
const char *typname;
1193511931
const char *argmode;
1193611932
const char *argname;
1193711933

@@ -11970,7 +11966,6 @@ format_function_arguments_old(Archive *fout,
1197011966
argname ? fmtId(argname) : "",
1197111967
argname ? " " : "",
1197211968
typname);
11973-
free(typname);
1197411969
}
1197511970
appendPQExpBufferChar(&fn, ')');
1197611971
return fn.data;
@@ -11999,15 +11994,12 @@ format_function_signature(Archive *fout, const FuncInfo *finfo, bool honor_quote
1199911994
appendPQExpBuffer(&fn, "%s(", finfo->dobj.name);
1200011995
for (j = 0; j < finfo->nargs; j++)
1200111996
{
12002-
char *typname;
12003-
1200411997
if (j > 0)
1200511998
appendPQExpBufferStr(&fn, ", ");
1200611999

12007-
typname = getFormattedTypeName(fout, finfo->argtypes[j],
12008-
zeroIsError);
12009-
appendPQExpBufferStr(&fn, typname);
12010-
free(typname);
12000+
appendPQExpBufferStr(&fn,
12001+
getFormattedTypeName(fout, finfo->argtypes[j],
12002+
zeroIsError));
1201112003
}
1201212004
appendPQExpBufferChar(&fn, ')');
1201312005
return fn.data;
@@ -12052,7 +12044,6 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1205212044
char *prosupport;
1205312045
char *proparallel;
1205412046
char *lanname;
12055-
char *rettypename;
1205612047
int nallargs;
1205712048
char **allargtypes = NULL;
1205812049
char **argmodes = NULL;
@@ -12342,14 +12333,10 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1234212333
else if (funcresult)
1234312334
appendPQExpBuffer(q, " RETURNS %s", funcresult);
1234412335
else
12345-
{
12346-
rettypename = getFormattedTypeName(fout, finfo->prorettype,
12347-
zeroIsError);
1234812336
appendPQExpBuffer(q, " RETURNS %s%s",
1234912337
(proretset[0] == 't') ? "SETOF " : "",
12350-
rettypename);
12351-
free(rettypename);
12352-
}
12338+
getFormattedTypeName(fout, finfo->prorettype,
12339+
zeroIsError));
1235312340

1235412341
appendPQExpBuffer(q, "\n LANGUAGE %s", fmtId(lanname));
1235512342

@@ -12556,8 +12543,8 @@ dumpCast(Archive *fout, const CastInfo *cast)
1255612543
PQExpBuffer labelq;
1255712544
PQExpBuffer castargs;
1255812545
FuncInfo *funcInfo = NULL;
12559-
char *sourceType;
12560-
char *targetType;
12546+
const char *sourceType;
12547+
const char *targetType;
1256112548

1256212549
/* Skip if not to be dumped */
1256312550
if (!cast->dobj.dump || dopt->dataOnly)
@@ -12643,9 +12630,6 @@ dumpCast(Archive *fout, const CastInfo *cast)
1264312630
NULL, "",
1264412631
cast->dobj.catId, 0, cast->dobj.dumpId);
1264512632

12646-
free(sourceType);
12647-
free(targetType);
12648-
1264912633
destroyPQExpBuffer(defqry);
1265012634
destroyPQExpBuffer(delqry);
1265112635
destroyPQExpBuffer(labelq);
@@ -12666,7 +12650,7 @@ dumpTransform(Archive *fout, const TransformInfo *transform)
1266612650
FuncInfo *fromsqlFuncInfo = NULL;
1266712651
FuncInfo *tosqlFuncInfo = NULL;
1266812652
char *lanname;
12669-
char *transformType;
12653+
const char *transformType;
1267012654

1267112655
/* Skip if not to be dumped */
1267212656
if (!transform->dobj.dump || dopt->dataOnly)
@@ -12773,7 +12757,6 @@ dumpTransform(Archive *fout, const TransformInfo *transform)
1277312757
transform->dobj.catId, 0, transform->dobj.dumpId);
1277412758

1277512759
free(lanname);
12776-
free(transformType);
1277712760
destroyPQExpBuffer(defqry);
1277812761
destroyPQExpBuffer(delqry);
1277912762
destroyPQExpBuffer(labelq);
@@ -14071,17 +14054,11 @@ format_aggregate_signature(const AggInfo *agginfo, Archive *fout, bool honor_quo
1407114054
{
1407214055
appendPQExpBufferChar(&buf, '(');
1407314056
for (j = 0; j < agginfo->aggfn.nargs; j++)
14074-
{
14075-
char *typname;
14076-
14077-
typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j],
14078-
zeroIsError);
14079-
1408014057
appendPQExpBuffer(&buf, "%s%s",
1408114058
(j > 0) ? ", " : "",
14082-
typname);
14083-
free(typname);
14084-
}
14059+
getFormattedTypeName(fout,
14060+
agginfo->aggfn.argtypes[j],
14061+
zeroIsError));
1408514062
appendPQExpBufferChar(&buf, ')');
1408614063
}
1408714064
return buf.data;
@@ -18754,8 +18731,10 @@ findDumpableDependencies(ArchiveHandle *AH, const DumpableObject *dobj,
1875418731
*
1875518732
* This does not guarantee to schema-qualify the output, so it should not
1875618733
* be used to create the target object name for CREATE or ALTER commands.
18734+
*
18735+
* Note that the result is cached and must not be freed by the caller.
1875718736
*/
18758-
static char *
18737+
static const char *
1875918738
getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1876018739
{
1876118740
TypeInfo *typeInfo;
@@ -18766,15 +18745,15 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1876618745
if (oid == 0)
1876718746
{
1876818747
if ((opts & zeroAsStar) != 0)
18769-
return pg_strdup("*");
18748+
return "*";
1877018749
else if ((opts & zeroAsNone) != 0)
18771-
return pg_strdup("NONE");
18750+
return "NONE";
1877218751
}
1877318752

1877418753
/* see if we have the result cached in the type's TypeInfo record */
1877518754
typeInfo = findTypeByOid(oid);
1877618755
if (typeInfo && typeInfo->ftypname)
18777-
return pg_strdup(typeInfo->ftypname);
18756+
return typeInfo->ftypname;
1877818757

1877918758
query = createPQExpBuffer();
1878018759
appendPQExpBuffer(query, "SELECT pg_catalog.format_type('%u'::pg_catalog.oid, NULL)",
@@ -18788,9 +18767,14 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
1878818767
PQclear(res);
1878918768
destroyPQExpBuffer(query);
1879018769

18791-
/* cache a copy for later requests */
18770+
/*
18771+
* Cache the result for re-use in later requests, if possible. If we
18772+
* don't have a TypeInfo for the type, the string will be leaked once the
18773+
* caller is done with it ... but that case really should not happen, so
18774+
* leaking if it does seems acceptable.
18775+
*/
1879218776
if (typeInfo)
18793-
typeInfo->ftypname = pg_strdup(result);
18777+
typeInfo->ftypname = result;
1879418778

1879518779
return result;
1879618780
}

0 commit comments

Comments
 (0)