]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Avoid useless malloc/free traffic around getFormattedTypeName().
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 8 Sep 2021 19:09:43 +0000 (15:09 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 8 Sep 2021 19:09:43 +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 2c4763d9eabc857bfed018e1fb94c54638ddd08d..c0617076f24a332a5f5ae6aded779c38ababce42 100644 (file)
@@ -251,7 +251,7 @@ static char *getFormattedOperatorName(Archive *fout, const char *oproid);
 static char *convertTSFunction(Archive *fout, Oid funcOid);
 static Oid     findLastBuiltinOid_V71(Archive *fout, const char *);
 static Oid     findLastBuiltinOid_V70(Archive *fout);
-static char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
+static const char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
 static char *myFormatType(const char *typname, int32 typmod);
 static void getBlobs(Archive *fout);
 static void dumpBlob(Archive *fout, BlobInfo *binfo);
@@ -10488,13 +10488,9 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
        }
 
        if (OidIsValid(tyinfo->typelem))
-       {
-               char       *elemType;
-
-               elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroAsOpaque);
-               appendPQExpBuffer(q, ",\n    ELEMENT = %s", elemType);
-               free(elemType);
-       }
+               appendPQExpBuffer(q, ",\n    ELEMENT = %s",
+                                                 getFormattedTypeName(fout, tyinfo->typelem,
+                                                                                          zeroAsOpaque));
 
        if (strcmp(typcategory, "U") != 0)
        {
@@ -11296,7 +11292,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;
 
@@ -11335,7 +11331,6 @@ format_function_arguments_old(Archive *fout,
                                                  argname ? fmtId(argname) : "",
                                                  argname ? " " : "",
                                                  typname);
-               free(typname);
        }
        appendPQExpBufferChar(&fn, ')');
        return fn.data;
@@ -11364,15 +11359,12 @@ format_function_signature(Archive *fout, FuncInfo *finfo, bool honor_quotes)
                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],
-                                                                          zeroAsOpaque);
-               appendPQExpBufferStr(&fn, typname);
-               free(typname);
+               appendPQExpBufferStr(&fn,
+                                                        getFormattedTypeName(fout, finfo->argtypes[j],
+                                                                                                 zeroAsOpaque));
        }
        appendPQExpBufferChar(&fn, ')');
        return fn.data;
@@ -11415,7 +11407,6 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
        char       *prorows;
        char       *proparallel;
        char       *lanname;
-       char       *rettypename;
        int                     nallargs;
        char      **allargtypes = NULL;
        char      **argmodes = NULL;
@@ -11774,14 +11765,10 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
        if (funcresult)
                appendPQExpBuffer(q, "RETURNS %s", funcresult);
        else
-       {
-               rettypename = getFormattedTypeName(fout, finfo->prorettype,
-                                                                                  zeroAsOpaque);
                appendPQExpBuffer(q, "RETURNS %s%s",
                                                  (proretset[0] == 't') ? "SETOF " : "",
-                                                 rettypename);
-               free(rettypename);
-       }
+                                                 getFormattedTypeName(fout, finfo->prorettype,
+                                                                                          zeroAsOpaque));
 
        appendPQExpBuffer(q, "\n    LANGUAGE %s", fmtId(lanname));
 
@@ -11983,8 +11970,8 @@ dumpCast(Archive *fout, 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)
@@ -12071,9 +12058,6 @@ dumpCast(Archive *fout, CastInfo *cast)
                                        NULL, "",
                                        cast->dobj.catId, 0, cast->dobj.dumpId);
 
-       free(sourceType);
-       free(targetType);
-
        destroyPQExpBuffer(defqry);
        destroyPQExpBuffer(delqry);
        destroyPQExpBuffer(labelq);
@@ -12094,7 +12078,7 @@ dumpTransform(Archive *fout, 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)
@@ -12200,7 +12184,6 @@ dumpTransform(Archive *fout, TransformInfo *transform)
                                        transform->dobj.catId, 0, transform->dobj.dumpId);
 
        free(lanname);
-       free(transformType);
        destroyPQExpBuffer(defqry);
        destroyPQExpBuffer(delqry);
        destroyPQExpBuffer(labelq);
@@ -13480,17 +13463,11 @@ format_aggregate_signature(AggInfo *agginfo, Archive *fout, bool honor_quotes)
        {
                appendPQExpBufferChar(&buf, '(');
                for (j = 0; j < agginfo->aggfn.nargs; j++)
-               {
-                       char       *typname;
-
-                       typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j],
-                                                                                  zeroAsOpaque);
-
                        appendPQExpBuffer(&buf, "%s%s",
                                                          (j > 0) ? ", " : "",
-                                                         typname);
-                       free(typname);
-               }
+                                                         getFormattedTypeName(fout,
+                                                                                                  agginfo->aggfn.argtypes[j],
+                                                                                                  zeroAsOpaque));
                appendPQExpBufferChar(&buf, ')');
        }
        return buf.data;
@@ -17891,8 +17868,10 @@ findDumpableDependencies(ArchiveHandle *AH, 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;
@@ -17903,19 +17882,19 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
        if (oid == 0)
        {
                if ((opts & zeroAsOpaque) != 0)
-                       return pg_strdup(g_opaque_type);
+                       return g_opaque_type;
                else if ((opts & zeroAsAny) != 0)
-                       return pg_strdup("'any'");
+                       return "'any'";
                else 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();
        if (fout->remoteVersion >= 70300)
@@ -17952,9 +17931,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;
 }