Avoid useless malloc/free traffic around getFormattedTypeName().
authorTom Lane <[email protected]>
Wed, 8 Sep 2021 19:09:42 +0000 (15:09 -0400)
committerTom Lane <[email protected]>
Wed, 8 Sep 2021 19:09:42 +0000 (15:09 -0400)
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 6c450a861 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.

src/bin/pg_dump/pg_dump.c

index 67be84982926d28242e344c7de87bdcb03709a9e..2febcd4213cda058d4707ccb450109552f05eb4e 100644 (file)
@@ -271,7 +271,7 @@ static char *convertRegProcReference(const char *proc);
 static char *getFormattedOperatorName(const char *oproid);
 static char *convertTSFunction(Archive *fout, Oid funcOid);
 static Oid     findLastBuiltinOid_V71(Archive *fout);
-static char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
+static const char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
 static void getBlobs(Archive *fout);
 static void dumpBlob(Archive *fout, const BlobInfo *binfo);
 static int     dumpBlobs(Archive *fout, const void *arg);
@@ -11252,13 +11252,9 @@ dumpBaseType(Archive *fout, const TypeInfo *tyinfo)
                appendPQExpBuffer(q, ",\n    SUBSCRIPT = %s", typsubscript);
 
        if (OidIsValid(tyinfo->typelem))
-       {
-               char       *elemType;
-
-               elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroIsError);
-               appendPQExpBuffer(q, ",\n    ELEMENT = %s", elemType);
-               free(elemType);
-       }
+               appendPQExpBuffer(q, ",\n    ELEMENT = %s",
+                                                 getFormattedTypeName(fout, tyinfo->typelem,
+                                                                                          zeroIsError));
 
        if (strcmp(typcategory, "U") != 0)
        {
@@ -12062,7 +12058,7 @@ format_function_arguments_old(Archive *fout,
        for (j = 0; j < nallargs; j++)
        {
                Oid                     typid;
-               char       *typname;
+               const char *typname;
                const char *argmode;
                const char *argname;
 
@@ -12101,7 +12097,6 @@ format_function_arguments_old(Archive *fout,
                                                  argname ? fmtId(argname) : "",
                                                  argname ? " " : "",
                                                  typname);
-               free(typname);
        }
        appendPQExpBufferChar(&fn, ')');
        return fn.data;
@@ -12130,15 +12125,12 @@ format_function_signature(Archive *fout, const FuncInfo *finfo, bool honor_quote
                appendPQExpBuffer(&fn, "%s(", finfo->dobj.name);
        for (j = 0; j < finfo->nargs; j++)
        {
-               char       *typname;
-
                if (j > 0)
                        appendPQExpBufferStr(&fn, ", ");
 
-               typname = getFormattedTypeName(fout, finfo->argtypes[j],
-                                                                          zeroIsError);
-               appendPQExpBufferStr(&fn, typname);
-               free(typname);
+               appendPQExpBufferStr(&fn,
+                                                        getFormattedTypeName(fout, finfo->argtypes[j],
+                                                                                                 zeroIsError));
        }
        appendPQExpBufferChar(&fn, ')');
        return fn.data;
@@ -12183,7 +12175,6 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
        char       *prosupport;
        char       *proparallel;
        char       *lanname;
-       char       *rettypename;
        int                     nallargs;
        char      **allargtypes = NULL;
        char      **argmodes = NULL;
@@ -12473,14 +12464,10 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
        else if (funcresult)
                appendPQExpBuffer(q, " RETURNS %s", funcresult);
        else
-       {
-               rettypename = getFormattedTypeName(fout, finfo->prorettype,
-                                                                                  zeroIsError);
                appendPQExpBuffer(q, " RETURNS %s%s",
                                                  (proretset[0] == 't') ? "SETOF " : "",
-                                                 rettypename);
-               free(rettypename);
-       }
+                                                 getFormattedTypeName(fout, finfo->prorettype,
+                                                                                          zeroIsError));
 
        appendPQExpBuffer(q, "\n    LANGUAGE %s", fmtId(lanname));
 
@@ -12687,8 +12674,8 @@ dumpCast(Archive *fout, const CastInfo *cast)
        PQExpBuffer labelq;
        PQExpBuffer castargs;
        FuncInfo   *funcInfo = NULL;
-       char       *sourceType;
-       char       *targetType;
+       const char *sourceType;
+       const char *targetType;
 
        /* Skip if not to be dumped */
        if (!cast->dobj.dump || dopt->dataOnly)
@@ -12774,9 +12761,6 @@ dumpCast(Archive *fout, const CastInfo *cast)
                                        NULL, "",
                                        cast->dobj.catId, 0, cast->dobj.dumpId);
 
-       free(sourceType);
-       free(targetType);
-
        destroyPQExpBuffer(defqry);
        destroyPQExpBuffer(delqry);
        destroyPQExpBuffer(labelq);
@@ -12797,7 +12781,7 @@ dumpTransform(Archive *fout, const TransformInfo *transform)
        FuncInfo   *fromsqlFuncInfo = NULL;
        FuncInfo   *tosqlFuncInfo = NULL;
        char       *lanname;
-       char       *transformType;
+       const char *transformType;
 
        /* Skip if not to be dumped */
        if (!transform->dobj.dump || dopt->dataOnly)
@@ -12904,7 +12888,6 @@ dumpTransform(Archive *fout, const TransformInfo *transform)
                                        transform->dobj.catId, 0, transform->dobj.dumpId);
 
        free(lanname);
-       free(transformType);
        destroyPQExpBuffer(defqry);
        destroyPQExpBuffer(delqry);
        destroyPQExpBuffer(labelq);
@@ -14202,17 +14185,11 @@ format_aggregate_signature(const AggInfo *agginfo, Archive *fout, bool honor_quo
        {
                appendPQExpBufferChar(&buf, '(');
                for (j = 0; j < agginfo->aggfn.nargs; j++)
-               {
-                       char       *typname;
-
-                       typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j],
-                                                                                  zeroIsError);
-
                        appendPQExpBuffer(&buf, "%s%s",
                                                          (j > 0) ? ", " : "",
-                                                         typname);
-                       free(typname);
-               }
+                                                         getFormattedTypeName(fout,
+                                                                                                  agginfo->aggfn.argtypes[j],
+                                                                                                  zeroIsError));
                appendPQExpBufferChar(&buf, ')');
        }
        return buf.data;
@@ -18885,8 +18862,10 @@ findDumpableDependencies(ArchiveHandle *AH, const DumpableObject *dobj,
  *
  * This does not guarantee to schema-qualify the output, so it should not
  * be used to create the target object name for CREATE or ALTER commands.
+ *
+ * Note that the result is cached and must not be freed by the caller.
  */
-static char *
+static const char *
 getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
 {
        TypeInfo   *typeInfo;
@@ -18897,15 +18876,15 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
        if (oid == 0)
        {
                if ((opts & zeroAsStar) != 0)
-                       return pg_strdup("*");
+                       return "*";
                else if ((opts & zeroAsNone) != 0)
-                       return pg_strdup("NONE");
+                       return "NONE";
        }
 
        /* see if we have the result cached in the type's TypeInfo record */
        typeInfo = findTypeByOid(oid);
        if (typeInfo && typeInfo->ftypname)
-               return pg_strdup(typeInfo->ftypname);
+               return typeInfo->ftypname;
 
        query = createPQExpBuffer();
        appendPQExpBuffer(query, "SELECT pg_catalog.format_type('%u'::pg_catalog.oid, NULL)",
@@ -18919,9 +18898,14 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
        PQclear(res);
        destroyPQExpBuffer(query);
 
-       /* cache a copy for later requests */
+       /*
+        * Cache the result for re-use in later requests, if possible.  If we
+        * don't have a TypeInfo for the type, the string will be leaked once the
+        * caller is done with it ... but that case really should not happen, so
+        * leaking if it does seems acceptable.
+        */
        if (typeInfo)
-               typeInfo->ftypname = pg_strdup(result);
+               typeInfo->ftypname = result;
 
        return result;
 }