Hi all,
Please find attached a set of fixes for a couple of things in src/bin:
- pg_dump/pg_dumpall:
-- getFormattedTypeName, convertTSFunction and myFormatType return
strdup'd results that are never free'd.
-- convertTSFunction returns const char. I fail to see the point of
that... In my opinion we are fine with just returning a char pointer,
which is strdup'd so as it can be freed by the caller.
- initdb's and pg_regress' use getaddrinfo, but do not free the
returned result with freeaddrinfo().
- Coverity noticed on the way some leaked memory in pg_upgrade's
equivalent_locale().
Those issues have been mostly spotted by Coverity, I may have spotted
some of them while looking at similar code paths... In any case that's
Coverity's win ;)
Regards,
--
Michael
From 7898cecd7ebf6e729a0a33b14b1d7d0512314bd1 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Mon, 8 Jun 2015 14:52:28 +0900
Subject: [PATCH 1/2] Fix some memory leaks in pg_dump and pg_dumpall
---
src/bin/pg_dump/pg_dump.c | 83 ++++++++++++++++++++++++++++----------------
src/bin/pg_dump/pg_dumpall.c | 7 ++++
2 files changed, 60 insertions(+), 30 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a72dfe9..7458389 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -225,7 +225,7 @@ static char *format_function_signature(Archive *fout,
static char *convertRegProcReference(Archive *fout,
const char *proc);
static char *convertOperatorReference(Archive *fout, const char *opr);
-static const char *convertTSFunction(Archive *fout, Oid funcOid);
+static char *convertTSFunction(Archive *fout, Oid funcOid);
static Oid findLastBuiltinOid_V71(Archive *fout, const char *);
static Oid findLastBuiltinOid_V70(Archive *fout);
static void selectSourceSchema(Archive *fout, const char *schemaName);
@@ -10454,10 +10454,12 @@ dumpFunc(Archive *fout, DumpOptions *dopt, FuncInfo *finfo)
parseOidArray(protrftypes, typeids, FUNC_MAX_ARGS);
for (i = 0; typeids[i]; i++)
{
+ char *elemType;
if (i != 0)
appendPQExpBufferStr(q, ", ");
- appendPQExpBuffer(q, "FOR TYPE %s",
- getFormattedTypeName(fout, typeids[i], zeroAsNone));
+ elemType = getFormattedTypeName(fout, typeids[i], zeroAsNone);
+ appendPQExpBuffer(q, "FOR TYPE %s", elemType);
+ free(elemType);
}
}
@@ -10593,6 +10595,8 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
PQExpBuffer delqry;
PQExpBuffer labelq;
FuncInfo *funcInfo = NULL;
+ char *sourceType;
+ char *targetType;
/* Skip if not to be dumped */
if (!cast->dobj.dump || dopt->dataOnly)
@@ -10616,13 +10620,13 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
delqry = createPQExpBuffer();
labelq = createPQExpBuffer();
+ sourceType = getFormattedTypeName(fout, cast->castsource, zeroAsNone);
+ targetType = getFormattedTypeName(fout, cast->casttarget, zeroAsNone);
appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n",
- getFormattedTypeName(fout, cast->castsource, zeroAsNone),
- getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
+ sourceType, targetType);
appendPQExpBuffer(defqry, "CREATE CAST (%s AS %s) ",
- getFormattedTypeName(fout, cast->castsource, zeroAsNone),
- getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
+ sourceType, targetType);
switch (cast->castmethod)
{
@@ -10660,8 +10664,7 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
appendPQExpBufferStr(defqry, ";\n");
appendPQExpBuffer(labelq, "CAST (%s AS %s)",
- getFormattedTypeName(fout, cast->castsource, zeroAsNone),
- getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
+ sourceType, targetType);
if (dopt->binary_upgrade)
binary_upgrade_extension_member(defqry, &cast->dobj, labelq->data);
@@ -10679,6 +10682,9 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
NULL, "",
cast->dobj.catId, 0, cast->dobj.dumpId);
+ free(sourceType);
+ free(targetType);
+
destroyPQExpBuffer(defqry);
destroyPQExpBuffer(delqry);
destroyPQExpBuffer(labelq);
@@ -10696,6 +10702,7 @@ dumpTransform(Archive *fout, DumpOptions *dopt, TransformInfo *transform)
FuncInfo *fromsqlFuncInfo = NULL;
FuncInfo *tosqlFuncInfo = NULL;
char *lanname;
+ char *transformType;
/* Skip if not to be dumped */
if (!transform->dobj.dump || dopt->dataOnly)
@@ -10723,13 +10730,14 @@ dumpTransform(Archive *fout, DumpOptions *dopt, TransformInfo *transform)
labelq = createPQExpBuffer();
lanname = get_language_name(fout, transform->trflang);
+ transformType = getFormattedTypeName(fout, transform->trftype, zeroAsNone);
appendPQExpBuffer(delqry, "DROP TRANSFORM FOR %s LANGUAGE %s;\n",
- getFormattedTypeName(fout, transform->trftype, zeroAsNone),
+ transformType,
lanname);
appendPQExpBuffer(defqry, "CREATE TRANSFORM FOR %s LANGUAGE %s (",
- getFormattedTypeName(fout, transform->trftype, zeroAsNone),
+ transformType,
lanname);
if (!transform->trffromsql && !transform->trftosql)
@@ -10777,7 +10785,7 @@ dumpTransform(Archive *fout, DumpOptions *dopt, TransformInfo *transform)
appendPQExpBuffer(defqry, ");\n");
appendPQExpBuffer(labelq, "TRANSFORM FOR %s LANGUAGE %s",
- getFormattedTypeName(fout, transform->trftype, zeroAsNone),
+ transformType,
lanname);
if (dopt->binary_upgrade)
@@ -10797,6 +10805,7 @@ dumpTransform(Archive *fout, DumpOptions *dopt, TransformInfo *transform)
transform->dobj.catId, 0, transform->dobj.dumpId);
free(lanname);
+ free(transformType);
destroyPQExpBuffer(defqry);
destroyPQExpBuffer(delqry);
destroyPQExpBuffer(labelq);
@@ -11172,7 +11181,7 @@ convertOperatorReference(Archive *fout, const char *opr)
* caller should ensure we are in the proper schema, because the results
* are search path dependent!
*/
-static const char *
+static char *
convertTSFunction(Archive *fout, Oid funcOid)
{
char *result;
@@ -12499,6 +12508,7 @@ dumpTSParser(Archive *fout, DumpOptions *dopt, TSParserInfo *prsinfo)
PQExpBuffer q;
PQExpBuffer delq;
PQExpBuffer labelq;
+ char *buf;
/* Skip if not to be dumped */
if (!prsinfo->dobj.dump || dopt->dataOnly)
@@ -12514,17 +12524,24 @@ dumpTSParser(Archive *fout, DumpOptions *dopt, TSParserInfo *prsinfo)
appendPQExpBuffer(q, "CREATE TEXT SEARCH PARSER %s (\n",
fmtId(prsinfo->dobj.name));
- appendPQExpBuffer(q, " START = %s,\n",
- convertTSFunction(fout, prsinfo->prsstart));
- appendPQExpBuffer(q, " GETTOKEN = %s,\n",
- convertTSFunction(fout, prsinfo->prstoken));
- appendPQExpBuffer(q, " END = %s,\n",
- convertTSFunction(fout, prsinfo->prsend));
+ buf = convertTSFunction(fout, prsinfo->prsstart);
+ appendPQExpBuffer(q, " START = %s,\n", buf);
+ free(buf);
+ buf = convertTSFunction(fout, prsinfo->prstoken);
+ appendPQExpBuffer(q, " GETTOKEN = %s,\n", buf);
+ free(buf);
+ buf = convertTSFunction(fout, prsinfo->prsend);
+ appendPQExpBuffer(q, " END = %s,\n", buf);
+ free(buf);
if (prsinfo->prsheadline != InvalidOid)
- appendPQExpBuffer(q, " HEADLINE = %s,\n",
- convertTSFunction(fout, prsinfo->prsheadline));
- appendPQExpBuffer(q, " LEXTYPES = %s );\n",
- convertTSFunction(fout, prsinfo->prslextype));
+ {
+ buf = convertTSFunction(fout, prsinfo->prsheadline);
+ appendPQExpBuffer(q, " HEADLINE = %s,\n", buf);
+ free(buf);
+ }
+ buf = convertTSFunction(fout, prsinfo->prslextype);
+ appendPQExpBuffer(q, " LEXTYPES = %s );\n", buf);
+ free(buf);
/*
* DROP must be fully qualified in case same name appears in pg_catalog
@@ -12658,6 +12675,7 @@ dumpTSTemplate(Archive *fout, DumpOptions *dopt, TSTemplateInfo *tmplinfo)
PQExpBuffer q;
PQExpBuffer delq;
PQExpBuffer labelq;
+ char *buf;
/* Skip if not to be dumped */
if (!tmplinfo->dobj.dump || dopt->dataOnly)
@@ -12674,10 +12692,14 @@ dumpTSTemplate(Archive *fout, DumpOptions *dopt, TSTemplateInfo *tmplinfo)
fmtId(tmplinfo->dobj.name));
if (tmplinfo->tmplinit != InvalidOid)
- appendPQExpBuffer(q, " INIT = %s,\n",
- convertTSFunction(fout, tmplinfo->tmplinit));
- appendPQExpBuffer(q, " LEXIZE = %s );\n",
- convertTSFunction(fout, tmplinfo->tmpllexize));
+ {
+ buf = convertTSFunction(fout, tmplinfo->tmplinit);
+ appendPQExpBuffer(q, " INIT = %s,\n", buf);
+ free(buf);
+ }
+ buf = convertTSFunction(fout, tmplinfo->tmpllexize);
+ appendPQExpBuffer(q, " LEXIZE = %s );\n", buf);
+ free(buf);
/*
* DROP must be fully qualified in case same name appears in pg_catalog
@@ -13876,10 +13898,11 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
}
else
{
+ char *buf = myFormatType(tbinfo->atttypnames[j],
+ tbinfo->atttypmod[j]);
/* If no format_type, fake it */
- appendPQExpBuffer(q, " %s",
- myFormatType(tbinfo->atttypnames[j],
- tbinfo->atttypmod[j]));
+ appendPQExpBuffer(q, " %s", buf);
+ free(buf);
}
/* Add collation if not default for the type */
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index d98c83e..c4b6ae8 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1442,6 +1442,13 @@ dumpCreateDB(PGconn *conn)
free(fdbname);
}
+ if (default_encoding)
+ free(default_encoding);
+ if (default_collate)
+ free(default_collate);
+ if (default_ctype)
+ free(default_ctype);
+
PQclear(res);
destroyPQExpBuffer(buf);
--
2.4.3
From 6f5515e4613c3f80a049bf24f2dd9366ae3575ea Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Mon, 8 Jun 2015 15:46:27 +0900
Subject: [PATCH 2/2] Additional leak fixes for src/bin stuff
The result of getaddrinfo() should be freed with freeaddrinfo.
---
src/bin/initdb/initdb.c | 5 ++++-
src/bin/pg_upgrade/check.c | 6 ++++++
src/test/regress/pg_regress.c | 4 +++-
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index feeff9e..5bda707 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1359,7 +1359,7 @@ setup_config(void)
* have to run on a machine without.
*/
{
- struct addrinfo *gai_result;
+ struct addrinfo *gai_result = NULL;
struct addrinfo hints;
int err = 0;
@@ -1385,6 +1385,8 @@ setup_config(void)
conflines = replace_token(conflines,
"host all all ::1",
"#host all all ::1");
+ if (gai_result)
+ freeaddrinfo(gai_result);
}
#else /* !HAVE_IPV6 */
/* If we didn't compile IPV6 support at all, always comment it out */
@@ -2191,6 +2193,7 @@ set_info_version(void)
micro = strtol(endptr + 1, &endptr, 10);
snprintf(infoversion, sizeof(infoversion), "%02ld.%02ld.%04ld%s",
major, minor, micro, letterversion);
+ pg_free(vstr);
}
/*
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 5a91871..41d4606 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -337,8 +337,14 @@ equivalent_locale(int category, const char *loca, const char *locb)
lenb = charb ? (charb - canonb) : strlen(canonb);
if (lena == lenb && pg_strncasecmp(canona, canonb, lena) == 0)
+ {
+ pg_free(canona);
+ pg_free(canonb);
return true;
+ }
+ pg_free(canona);
+ pg_free(canonb);
return false;
}
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index a267894..6bfb498 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -969,7 +969,7 @@ config_sspi_auth(const char *pgdata)
* ::1 (IPv6 loopback) as a numeric host address string.
*/
{
- struct addrinfo *gai_result;
+ struct addrinfo *gai_result = NULL;
struct addrinfo hints;
WSADATA wsaData;
@@ -984,6 +984,8 @@ config_sspi_auth(const char *pgdata)
have_ipv6 = (WSAStartup(MAKEWORD(2, 2), &wsaData) == 0 &&
getaddrinfo("::1", NULL, &hints, &gai_result) == 0);
+ if (gai_result)
+ freeaddrinfo(gai_result);
}
/* Check a Write outcome and report any error. */
--
2.4.3
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
