Memory leak fix in psql

Started by tanghy.fnst@fujitsu.comover 3 years ago28 messages

over 3 years ago

1 attachment(s)

I think there is a newly introduced memory leak in your patch d2d3547.
Try to fix it in the attached patch.
Kindly to have a check.

Regards,
Tang

Attachments:

v1-0001-fix-the-memory-leak-in-validateSQLNamePattern.patchapplication/octet-stream; name=v1-0001-fix-the-memory-leak-in-validateSQLNamePattern.patchDownload

From 76e85b22504e34d15e4102631f9caba755c43869 Mon Sep 17 00:00:00 2001
From: tanghy <tanghy.fnst@fujitsu.com>
Date: Tue, 19 Jul 2022 17:15:05 +0900
Subject: [PATCH v1] fix the memory leak in validateSQLNamePattern


diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 88d92a08ae..0ce38e4b4c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6023,15 +6023,18 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 		if (PQdb(pset.db) == NULL)
 		{
 			pg_log_error("You are currently not connected to a database.");
+			termPQExpBuffer(&dbbuf);
 			return false;
 		}
 		if (strcmp(PQdb(pset.db), dbbuf.data) != 0)
 		{
 			pg_log_error("cross-database references are not implemented: %s",
 						 pattern);
+			termPQExpBuffer(&dbbuf);
 			return false;
 		}
 	}
+	termPQExpBuffer(&dbbuf);
 	return true;
 }
 
-- 
2.33.0.windows.2

Japin Li

japinli@hotmail.com

over 3 years ago

In reply to: tanghy.fnst@fujitsu.com (#1)

1 attachment(s)

Re: Memory leak fix in psql

On Tue, 19 Jul 2022 at 17:02, tanghy.fnst@fujitsu.com <tanghy.fnst@fujitsu.com> wrote:

Hi

I think there is a newly introduced memory leak in your patch d2d3547.
Try to fix it in the attached patch.
Kindly to have a check.

Yeah, it leaks, and the patch can fix it.

After looking around, I found psql/describe.c also has some memory leaks,
attached a patch to fix these leaks.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Attachments:

v1-0001-Fix-the-memory-leak-in-psql-describe.patchtext/x-patchDownload

From 3036a0986f8ff490c133930524e2d5f5104249ff Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Tue, 19 Jul 2022 18:27:25 +0800
Subject: [PATCH v1 1/1] Fix the memory leak in psql describe

---
 src/bin/psql/describe.c | 150 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 0ce38e4b4c..11a441f52f 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -112,7 +112,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
 
@@ -182,7 +185,10 @@ describeAccessMethods(const char *pattern, bool verbose)
 								NULL, "amname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -244,7 +250,10 @@ describeTablespaces(const char *pattern, bool verbose)
 								NULL, "spcname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -534,7 +543,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	for (int i = 0; i < num_arg_patterns; i++)
 	{
@@ -561,7 +573,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
+			{
+				termPQExpBuffer(&buf);
 				return false;
+			}
 		}
 		else
 		{
@@ -682,7 +697,10 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
 								"pg_catalog.format_type(t.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -836,7 +854,10 @@ describeOperators(const char *oper_pattern,
 								"n.nspname", "o.oprname", NULL,
 								"pg_catalog.pg_operator_is_visible(o.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	if (num_arg_patterns == 1)
 		appendPQExpBufferStr(&buf, "  AND o.oprleft = 0\n");
@@ -866,7 +887,10 @@ describeOperators(const char *oper_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
+			{
+				termPQExpBuffer(&buf);
 				return false;
+			}
 		}
 		else
 		{
@@ -953,7 +977,10 @@ listAllDbs(const char *pattern, bool verbose)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "d.datname", NULL, NULL,
 									NULL, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 	res = PSQLexec(buf.data);
@@ -1106,7 +1133,10 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -1177,7 +1207,10 @@ listDefaultACLs(const char *pattern)
 								"pg_catalog.pg_get_userbyid(d.defaclrole)",
 								NULL,
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 3;");
 
@@ -1428,7 +1461,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2, 3;");
 
@@ -3614,7 +3650,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -3739,11 +3778,17 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 					  gettext_noop("Settings"));
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL, &havewhere, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 	if (!validateSQLNamePattern(&buf, pattern2, havewhere, false,
 								NULL, "d.datname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
@@ -3940,7 +3985,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
 
@@ -4157,7 +4205,10 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
 					  mixed_output ? "\"Type\" DESC, " : "",
@@ -4233,7 +4284,10 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "l.lanname", NULL, NULL,
 									NULL, 2))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf, "WHERE l.lanplcallfoid != 0\n");
@@ -4319,7 +4373,10 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "t.typname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4395,7 +4452,10 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.conname", NULL,
 								"pg_catalog.pg_conversion_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4542,7 +4602,10 @@ listEventTriggers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "evtname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1");
 
@@ -4638,7 +4701,10 @@ listExtendedStats(const char *pattern)
 								"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text", "es.stxname",
 								NULL, "pg_catalog.pg_statistics_obj_is_visible(es.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4742,7 +4808,10 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(ts.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(ts.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, ") OR (true");
 
@@ -4751,7 +4820,10 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(tt.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(tt.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, ") )\nORDER BY 1, 2;");
 
@@ -4851,7 +4923,10 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.collname", NULL,
 								"pg_catalog.pg_collation_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4914,7 +4989,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5029,7 +5107,10 @@ listTSParsers(const char *pattern, bool verbose)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5072,7 +5153,10 @@ listTSParsersVerbose(const char *pattern)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5215,7 +5299,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
@@ -5281,7 +5368,10 @@ listTSDictionaries(const char *pattern, bool verbose)
 								"n.nspname", "d.dictname", NULL,
 								"pg_catalog.pg_ts_dict_is_visible(d.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5344,7 +5434,10 @@ listTSTemplates(const char *pattern, bool verbose)
 								"n.nspname", "t.tmplname", NULL,
 								"pg_catalog.pg_ts_template_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5396,7 +5489,10 @@ listTSConfigs(const char *pattern, bool verbose)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5440,7 +5536,10 @@ listTSConfigsVerbose(const char *pattern)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 3, 2;");
 
@@ -5613,7 +5712,10 @@ listForeignDataWrappers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "fdwname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5687,7 +5789,10 @@ listForeignServers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "s.srvname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5740,7 +5845,10 @@ listUserMappings(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "um.srvname", "um.usename", NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5810,7 +5918,10 @@ listForeignTables(const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5859,7 +5970,10 @@ listExtensions(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5900,7 +6014,10 @@ listExtensionContents(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6093,7 +6210,10 @@ listPublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6208,7 +6328,10 @@ describePublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2;");
 
@@ -6420,7 +6543,10 @@ describeSubscriptions(const char *pattern, bool verbose)
 								NULL, "subname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6524,7 +6650,10 @@ listOperatorClasses(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (type_pattern)
 	{
 		/* Match type name pattern against either internal or external name */
@@ -6533,7 +6662,10 @@ listOperatorClasses(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
@@ -6600,7 +6732,10 @@ listOperatorFamilies(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (type_pattern)
 	{
 		appendPQExpBuffer(&buf,
@@ -6617,7 +6752,10 @@ listOperatorFamilies(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 		appendPQExpBufferStr(&buf, "  )\n");
 	}
 
@@ -6699,13 +6837,19 @@ listOpFamilyOperators(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"nsf.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  o.amoplefttype = o.amoprighttype DESC,\n"
@@ -6787,12 +6931,18 @@ listOpFamilyFunctions(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"ns.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  ap.amproclefttype = ap.amprocrighttype DESC,\n"
-- 
2.17.1

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: Japin Li (#2)

Re: Memory leak fix in psql

On Tue, Jul 19, 2022 at 06:41:13PM +0800, Japin Li wrote:

After looking around, I found psql/describe.c also has some memory leaks,
attached a patch to fix these leaks.

Indeed. There are quite a bit of them, so let's fix all that. You
have missed a couple of code paths in objectDescription().
--
Michael

Japin Li

japinli@hotmail.com

over 3 years ago

In reply to: Michael Paquier (#3)

1 attachment(s)

Re: Memory leak fix in psql

On Tue, 19 Jul 2022 at 20:32, Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Jul 19, 2022 at 06:41:13PM +0800, Japin Li wrote:

After looking around, I found psql/describe.c also has some memory leaks,
attached a patch to fix these leaks.

Indeed. There are quite a bit of them, so let's fix all that. You
have missed a couple of code paths in objectDescription().

Thanks for reviewing. Attached fix the memory leak in objectDescription().
Please consider v2 for further review.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Attachments:

v2-0001-Fix-the-memory-leak-in-psql-describe.patchtext/x-patchDownload

From b2bcc3a1bac67b8b414f2025607f8dd35e096289 Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Tue, 19 Jul 2022 18:27:25 +0800
Subject: [PATCH v2 1/1] Fix the memory leak in psql describe

---
 src/bin/psql/describe.c | 168 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 0ce38e4b4c..7a070a6cd0 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -112,7 +112,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
 
@@ -182,7 +185,10 @@ describeAccessMethods(const char *pattern, bool verbose)
 								NULL, "amname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -244,7 +250,10 @@ describeTablespaces(const char *pattern, bool verbose)
 								NULL, "spcname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -534,7 +543,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	for (int i = 0; i < num_arg_patterns; i++)
 	{
@@ -561,7 +573,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
+			{
+				termPQExpBuffer(&buf);
 				return false;
+			}
 		}
 		else
 		{
@@ -682,7 +697,10 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
 								"pg_catalog.format_type(t.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -836,7 +854,10 @@ describeOperators(const char *oper_pattern,
 								"n.nspname", "o.oprname", NULL,
 								"pg_catalog.pg_operator_is_visible(o.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	if (num_arg_patterns == 1)
 		appendPQExpBufferStr(&buf, "  AND o.oprleft = 0\n");
@@ -866,7 +887,10 @@ describeOperators(const char *oper_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
+			{
+				termPQExpBuffer(&buf);
 				return false;
+			}
 		}
 		else
 		{
@@ -953,7 +977,10 @@ listAllDbs(const char *pattern, bool verbose)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "d.datname", NULL, NULL,
 									NULL, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 	res = PSQLexec(buf.data);
@@ -1106,7 +1133,10 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -1177,7 +1207,10 @@ listDefaultACLs(const char *pattern)
 								"pg_catalog.pg_get_userbyid(d.defaclrole)",
 								NULL,
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 3;");
 
@@ -1253,7 +1286,10 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Domain constraint descriptions */
 	appendPQExpBuffer(&buf,
@@ -1277,7 +1313,10 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Operator class descriptions */
 	appendPQExpBuffer(&buf,
@@ -1301,7 +1340,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "o.opcname", NULL,
 								"pg_catalog.pg_opclass_is_visible(o.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Operator family descriptions */
 	appendPQExpBuffer(&buf,
@@ -1325,7 +1367,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "opf.opfname", NULL,
 								"pg_catalog.pg_opfamily_is_visible(opf.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Rule descriptions (ignore rules for views) */
 	appendPQExpBuffer(&buf,
@@ -1348,7 +1393,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "r.rulename", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Trigger descriptions */
 	appendPQExpBuffer(&buf,
@@ -1370,7 +1418,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "t.tgname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf,
 						 ") AS tt\n"
@@ -1428,7 +1479,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2, 3;");
 
@@ -3614,7 +3668,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -3739,11 +3796,17 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 					  gettext_noop("Settings"));
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL, &havewhere, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 	if (!validateSQLNamePattern(&buf, pattern2, havewhere, false,
 								NULL, "d.datname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
@@ -3940,7 +4003,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
 
@@ -4157,7 +4223,10 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
 					  mixed_output ? "\"Type\" DESC, " : "",
@@ -4233,7 +4302,10 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "l.lanname", NULL, NULL,
 									NULL, 2))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf, "WHERE l.lanplcallfoid != 0\n");
@@ -4319,7 +4391,10 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "t.typname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4395,7 +4470,10 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.conname", NULL,
 								"pg_catalog.pg_conversion_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4542,7 +4620,10 @@ listEventTriggers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "evtname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1");
 
@@ -4638,7 +4719,10 @@ listExtendedStats(const char *pattern)
 								"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text", "es.stxname",
 								NULL, "pg_catalog.pg_statistics_obj_is_visible(es.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4742,7 +4826,10 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(ts.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(ts.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, ") OR (true");
 
@@ -4751,7 +4838,10 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(tt.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(tt.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, ") )\nORDER BY 1, 2;");
 
@@ -4851,7 +4941,10 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.collname", NULL,
 								"pg_catalog.pg_collation_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4914,7 +5007,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5029,7 +5125,10 @@ listTSParsers(const char *pattern, bool verbose)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5072,7 +5171,10 @@ listTSParsersVerbose(const char *pattern)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5215,7 +5317,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
@@ -5281,7 +5386,10 @@ listTSDictionaries(const char *pattern, bool verbose)
 								"n.nspname", "d.dictname", NULL,
 								"pg_catalog.pg_ts_dict_is_visible(d.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5344,7 +5452,10 @@ listTSTemplates(const char *pattern, bool verbose)
 								"n.nspname", "t.tmplname", NULL,
 								"pg_catalog.pg_ts_template_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5396,7 +5507,10 @@ listTSConfigs(const char *pattern, bool verbose)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5440,7 +5554,10 @@ listTSConfigsVerbose(const char *pattern)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 3, 2;");
 
@@ -5613,7 +5730,10 @@ listForeignDataWrappers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "fdwname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5687,7 +5807,10 @@ listForeignServers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "s.srvname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5740,7 +5863,10 @@ listUserMappings(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "um.srvname", "um.usename", NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5810,7 +5936,10 @@ listForeignTables(const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5859,7 +5988,10 @@ listExtensions(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5900,7 +6032,10 @@ listExtensionContents(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6093,7 +6228,10 @@ listPublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6208,7 +6346,10 @@ describePublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2;");
 
@@ -6420,7 +6561,10 @@ describeSubscriptions(const char *pattern, bool verbose)
 								NULL, "subname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6524,7 +6668,10 @@ listOperatorClasses(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (type_pattern)
 	{
 		/* Match type name pattern against either internal or external name */
@@ -6533,7 +6680,10 @@ listOperatorClasses(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
@@ -6600,7 +6750,10 @@ listOperatorFamilies(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (type_pattern)
 	{
 		appendPQExpBuffer(&buf,
@@ -6617,7 +6770,10 @@ listOperatorFamilies(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 		appendPQExpBufferStr(&buf, "  )\n");
 	}
 
@@ -6699,13 +6855,19 @@ listOpFamilyOperators(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"nsf.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  o.amoplefttype = o.amoprighttype DESC,\n"
@@ -6787,12 +6949,18 @@ listOpFamilyFunctions(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"ns.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  ap.amproclefttype = ap.amprocrighttype DESC,\n"
-- 
2.17.1

Andres Freund

andres@anarazel.de

over 3 years ago

In reply to: Japin Li (#4)

Re: Memory leak fix in psql

Hi,

On 2022-07-19 21:08:53 +0800, Japin Li wrote:

From b2bcc3a1bac67b8b414f2025607f8dd35e096289 Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Tue, 19 Jul 2022 18:27:25 +0800
Subject: [PATCH v2 1/1] Fix the memory leak in psql describe

---
src/bin/psql/describe.c | 168 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 168 insertions(+)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 0ce38e4b4c..7a070a6cd0 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -112,7 +112,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
"n.nspname", "p.proname", NULL,
"pg_catalog.pg_function_is_visible(p.oid)",
NULL, 3))
+	{
+		termPQExpBuffer(&buf);
return false;
+	}
appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");

Adding copy over copy of this same block doesn't seem great. Can we instead
add a helper for it or such?

Greetings,

Andres Freund

Tom Lane

tgl@sss.pgh.pa.us

over 3 years ago

In reply to: Andres Freund (#5)

Re: Memory leak fix in psql

Andres Freund <andres@anarazel.de> writes:

On 2022-07-19 21:08:53 +0800, Japin Li wrote:
+	{
+		termPQExpBuffer(&buf);
return false;
+	}

Adding copy over copy of this same block doesn't seem great. Can we instead
add a helper for it or such?

The usual style in these files is something like

if (bad things happened)
goto fail;

...

fail:
termPQExpBuffer(&buf);
return false;

Yeah, it's old school, but please let's not have a few functions that
do it randomly differently from all their neighbors.

regards, tom lane

Mark Dilger

mark.dilger@enterprisedb.com

over 3 years ago

In reply to: tanghy.fnst@fujitsu.com (#1)

Re: Memory leak fix in psql

On Jul 19, 2022, at 2:02 AM, tanghy.fnst@fujitsu.com wrote:

I think there is a newly introduced memory leak in your patch d2d3547.

I agree. Thanks for noticing, and for the patch!

Try to fix it in the attached patch.
Kindly to have a check.

This looks ok, but comments down-thread seem reasonable, so I suspect a new patch will be needed. Would you like to author it, or would you prefer that I, as the guilty party, do so?

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: Tom Lane (#6)

Re: Memory leak fix in psql

On Tue, Jul 19, 2022 at 12:36:31PM -0400, Tom Lane wrote:

Yeah, it's old school, but please let's not have a few functions that
do it randomly differently from all their neighbors.

True enough. And it is not like we should free the PQExpBuffer given
by the caller in validateSQLNamePattern().
--
Michael

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: Mark Dilger (#7)

Re: Memory leak fix in psql

On Tue, Jul 19, 2022 at 09:43:21AM -0700, Mark Dilger wrote:

This looks ok, but comments down-thread seem reasonable, so I
suspect a new patch will be needed. Would you like to author it, or
would you prefer that I, as the guilty party, do so?

If any of you could update the patch, that would be great. Thanks!
--
Michael

#10

tanghy.fnst@fujitsu.com

over 3 years ago

In reply to: Japin Li (#2)

1 attachment(s)

RE: Memory leak fix in psql

On Tuesday, July 19, 2022 7:41 PM, Japin Li <japinli@hotmail.com> wrote:

After looking around, I found psql/describe.c also has some memory
leaks,
attached a patch to fix these leaks.

Thanks for your check and improvement.
Your fix LGTM so please allow me to merge it in the attached patch.
Based on your rebased version, now this new patch version is V3.

Regards,
Tang

Attachments:

v3-0001-fix-the-memory-leak-in-psql-describe.patchapplication/octet-stream; name=v3-0001-fix-the-memory-leak-in-psql-describe.patchDownload

From 464ec543011e529ccf8202ed0c6151805e78da8c Mon Sep 17 00:00:00 2001
From: tanghy <tanghy.fnst@fujitsu.com>
Date: Tue, 19 Jul 2022 17:15:05 +0900
Subject: [PATCH v3] fix the memory leak in psql describe


diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 88d92a08ae..7a070a6cd0 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -112,7 +112,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
 
@@ -182,7 +185,10 @@ describeAccessMethods(const char *pattern, bool verbose)
 								NULL, "amname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -244,7 +250,10 @@ describeTablespaces(const char *pattern, bool verbose)
 								NULL, "spcname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -534,7 +543,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	for (int i = 0; i < num_arg_patterns; i++)
 	{
@@ -561,7 +573,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
+			{
+				termPQExpBuffer(&buf);
 				return false;
+			}
 		}
 		else
 		{
@@ -682,7 +697,10 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
 								"pg_catalog.format_type(t.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -836,7 +854,10 @@ describeOperators(const char *oper_pattern,
 								"n.nspname", "o.oprname", NULL,
 								"pg_catalog.pg_operator_is_visible(o.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	if (num_arg_patterns == 1)
 		appendPQExpBufferStr(&buf, "  AND o.oprleft = 0\n");
@@ -866,7 +887,10 @@ describeOperators(const char *oper_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
+			{
+				termPQExpBuffer(&buf);
 				return false;
+			}
 		}
 		else
 		{
@@ -953,7 +977,10 @@ listAllDbs(const char *pattern, bool verbose)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "d.datname", NULL, NULL,
 									NULL, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 	res = PSQLexec(buf.data);
@@ -1106,7 +1133,10 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -1177,7 +1207,10 @@ listDefaultACLs(const char *pattern)
 								"pg_catalog.pg_get_userbyid(d.defaclrole)",
 								NULL,
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 3;");
 
@@ -1253,7 +1286,10 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Domain constraint descriptions */
 	appendPQExpBuffer(&buf,
@@ -1277,7 +1313,10 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Operator class descriptions */
 	appendPQExpBuffer(&buf,
@@ -1301,7 +1340,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "o.opcname", NULL,
 								"pg_catalog.pg_opclass_is_visible(o.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Operator family descriptions */
 	appendPQExpBuffer(&buf,
@@ -1325,7 +1367,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "opf.opfname", NULL,
 								"pg_catalog.pg_opfamily_is_visible(opf.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Rule descriptions (ignore rules for views) */
 	appendPQExpBuffer(&buf,
@@ -1348,7 +1393,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "r.rulename", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	/* Trigger descriptions */
 	appendPQExpBuffer(&buf,
@@ -1370,7 +1418,10 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "t.tgname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf,
 						 ") AS tt\n"
@@ -1428,7 +1479,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2, 3;");
 
@@ -3614,7 +3668,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -3739,11 +3796,17 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 					  gettext_noop("Settings"));
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL, &havewhere, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 	if (!validateSQLNamePattern(&buf, pattern2, havewhere, false,
 								NULL, "d.datname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
@@ -3940,7 +4003,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
 
@@ -4157,7 +4223,10 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
 					  mixed_output ? "\"Type\" DESC, " : "",
@@ -4233,7 +4302,10 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "l.lanname", NULL, NULL,
 									NULL, 2))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf, "WHERE l.lanplcallfoid != 0\n");
@@ -4319,7 +4391,10 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "t.typname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4395,7 +4470,10 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.conname", NULL,
 								"pg_catalog.pg_conversion_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4542,7 +4620,10 @@ listEventTriggers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "evtname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1");
 
@@ -4638,7 +4719,10 @@ listExtendedStats(const char *pattern)
 								"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text", "es.stxname",
 								NULL, "pg_catalog.pg_statistics_obj_is_visible(es.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4742,7 +4826,10 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(ts.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(ts.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, ") OR (true");
 
@@ -4751,7 +4838,10 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(tt.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(tt.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, ") )\nORDER BY 1, 2;");
 
@@ -4851,7 +4941,10 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.collname", NULL,
 								"pg_catalog.pg_collation_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4914,7 +5007,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5029,7 +5125,10 @@ listTSParsers(const char *pattern, bool verbose)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5072,7 +5171,10 @@ listTSParsersVerbose(const char *pattern)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5215,7 +5317,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
@@ -5281,7 +5386,10 @@ listTSDictionaries(const char *pattern, bool verbose)
 								"n.nspname", "d.dictname", NULL,
 								"pg_catalog.pg_ts_dict_is_visible(d.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5344,7 +5452,10 @@ listTSTemplates(const char *pattern, bool verbose)
 								"n.nspname", "t.tmplname", NULL,
 								"pg_catalog.pg_ts_template_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5396,7 +5507,10 @@ listTSConfigs(const char *pattern, bool verbose)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5440,7 +5554,10 @@ listTSConfigsVerbose(const char *pattern)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 3, 2;");
 
@@ -5613,7 +5730,10 @@ listForeignDataWrappers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "fdwname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5687,7 +5807,10 @@ listForeignServers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "s.srvname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5740,7 +5863,10 @@ listUserMappings(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "um.srvname", "um.usename", NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5810,7 +5936,10 @@ listForeignTables(const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5859,7 +5988,10 @@ listExtensions(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5900,7 +6032,10 @@ listExtensionContents(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6023,15 +6158,18 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 		if (PQdb(pset.db) == NULL)
 		{
 			pg_log_error("You are currently not connected to a database.");
+			termPQExpBuffer(&dbbuf);
 			return false;
 		}
 		if (strcmp(PQdb(pset.db), dbbuf.data) != 0)
 		{
 			pg_log_error("cross-database references are not implemented: %s",
 						 pattern);
+			termPQExpBuffer(&dbbuf);
 			return false;
 		}
 	}
+	termPQExpBuffer(&dbbuf);
 	return true;
 }
 
@@ -6090,7 +6228,10 @@ listPublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6205,7 +6346,10 @@ describePublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2;");
 
@@ -6417,7 +6561,10 @@ describeSubscriptions(const char *pattern, bool verbose)
 								NULL, "subname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6521,7 +6668,10 @@ listOperatorClasses(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (type_pattern)
 	{
 		/* Match type name pattern against either internal or external name */
@@ -6530,7 +6680,10 @@ listOperatorClasses(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
@@ -6597,7 +6750,10 @@ listOperatorFamilies(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (type_pattern)
 	{
 		appendPQExpBuffer(&buf,
@@ -6614,7 +6770,10 @@ listOperatorFamilies(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 		appendPQExpBufferStr(&buf, "  )\n");
 	}
 
@@ -6696,13 +6855,19 @@ listOpFamilyOperators(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"nsf.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  o.amoplefttype = o.amoprighttype DESC,\n"
@@ -6784,12 +6949,18 @@ listOpFamilyFunctions(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"ns.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  ap.amproclefttype = ap.amprocrighttype DESC,\n"
-- 
2.33.0.windows.2

#11

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: tanghy.fnst@fujitsu.com (#10)

1 attachment(s)

Re: Memory leak fix in psql

On Wed, Jul 20, 2022 at 03:14:35AM +0000, tanghy.fnst@fujitsu.com wrote:

Your fix LGTM so please allow me to merge it in the attached patch.
Based on your rebased version, now this new patch version is V3.

What about the argument of upthread where we could use a goto in
functions where there are multiple pattern validation checks? Per se
v4 attached.
--
Michael

Attachments:

v4-0001-fix-the-memory-leak-in-psql-describe.patchtext/x-diff; charset=us-asciiDownload

From ccc8bb83baf618ea1a481193403a9009cd4a24a5 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 20 Jul 2022 12:47:52 +0900
Subject: [PATCH v4] fix the memory leak in psql describe

---
 src/bin/psql/describe.c | 239 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 212 insertions(+), 27 deletions(-)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 88d92a08ae..1c40cc6d59 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -112,7 +112,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
 
@@ -182,7 +185,10 @@ describeAccessMethods(const char *pattern, bool verbose)
 								NULL, "amname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -244,7 +250,10 @@ describeTablespaces(const char *pattern, bool verbose)
 								NULL, "spcname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -534,7 +543,9 @@ describeFunctions(const char *functypes, const char *func_pattern,
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	for (int i = 0; i < num_arg_patterns; i++)
 	{
@@ -561,7 +572,9 @@ describeFunctions(const char *functypes, const char *func_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+			{
+				goto error_return;
+			}
 		}
 		else
 		{
@@ -599,6 +612,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -682,7 +699,10 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
 								"pg_catalog.format_type(t.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -836,7 +856,9 @@ describeOperators(const char *oper_pattern,
 								"n.nspname", "o.oprname", NULL,
 								"pg_catalog.pg_operator_is_visible(o.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	if (num_arg_patterns == 1)
 		appendPQExpBufferStr(&buf, "  AND o.oprleft = 0\n");
@@ -866,7 +888,9 @@ describeOperators(const char *oper_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+			{
+				goto error_return;
+			}
 		}
 		else
 		{
@@ -890,6 +914,10 @@ describeOperators(const char *oper_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -953,7 +981,10 @@ listAllDbs(const char *pattern, bool verbose)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "d.datname", NULL, NULL,
 									NULL, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 	res = PSQLexec(buf.data);
@@ -1106,7 +1137,10 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -1177,16 +1211,15 @@ listDefaultACLs(const char *pattern)
 								"pg_catalog.pg_get_userbyid(d.defaclrole)",
 								NULL,
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 3;");
 
 	res = PSQLexec(buf.data);
 	if (!res)
-	{
-		termPQExpBuffer(&buf);
-		return false;
-	}
+		goto error_return;
 
 	myopt.nullPrint = NULL;
 	printfPQExpBuffer(&buf, _("Default access privileges"));
@@ -1200,6 +1233,10 @@ listDefaultACLs(const char *pattern)
 	termPQExpBuffer(&buf);
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1253,7 +1290,9 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	/* Domain constraint descriptions */
 	appendPQExpBuffer(&buf,
@@ -1277,7 +1316,9 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	/* Operator class descriptions */
 	appendPQExpBuffer(&buf,
@@ -1301,7 +1342,9 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "o.opcname", NULL,
 								"pg_catalog.pg_opclass_is_visible(o.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	/* Operator family descriptions */
 	appendPQExpBuffer(&buf,
@@ -1325,7 +1368,9 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "opf.opfname", NULL,
 								"pg_catalog.pg_opfamily_is_visible(opf.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	/* Rule descriptions (ignore rules for views) */
 	appendPQExpBuffer(&buf,
@@ -1348,7 +1393,9 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "r.rulename", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	/* Trigger descriptions */
 	appendPQExpBuffer(&buf,
@@ -1370,7 +1417,9 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "t.tgname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	appendPQExpBufferStr(&buf,
 						 ") AS tt\n"
@@ -1393,6 +1442,10 @@ objectDescription(const char *pattern, bool showSystem)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1428,7 +1481,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2, 3;");
 
@@ -3614,7 +3670,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -3739,11 +3798,15 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 					  gettext_noop("Settings"));
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL, &havewhere, 1))
-		return false;
+	{
+		goto error_return;
+	}
 	if (!validateSQLNamePattern(&buf, pattern2, havewhere, false,
 								NULL, "d.datname", NULL, NULL,
 								NULL, 1))
-		return false;
+	{
+		goto error_return;
+	}
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
@@ -3779,6 +3842,10 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -3940,7 +4007,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
 
@@ -4157,7 +4227,10 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
 					  mixed_output ? "\"Type\" DESC, " : "",
@@ -4233,7 +4306,10 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "l.lanname", NULL, NULL,
 									NULL, 2))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf, "WHERE l.lanplcallfoid != 0\n");
@@ -4319,7 +4395,10 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "t.typname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4395,7 +4474,10 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.conname", NULL,
 								"pg_catalog.pg_conversion_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4542,7 +4624,10 @@ listEventTriggers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "evtname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1");
 
@@ -4638,7 +4723,10 @@ listExtendedStats(const char *pattern)
 								"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text", "es.stxname",
 								NULL, "pg_catalog.pg_statistics_obj_is_visible(es.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4742,7 +4830,9 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(ts.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(ts.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	appendPQExpBufferStr(&buf, ") OR (true");
 
@@ -4751,7 +4841,9 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(tt.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(tt.oid)",
 								NULL, 3))
-		return false;
+	{
+		goto error_return;
+	}
 
 	appendPQExpBufferStr(&buf, ") )\nORDER BY 1, 2;");
 
@@ -4770,6 +4862,10 @@ listCasts(const char *pattern, bool verbose)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -4851,7 +4947,10 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.collname", NULL,
 								"pg_catalog.pg_collation_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4914,7 +5013,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5029,7 +5131,10 @@ listTSParsers(const char *pattern, bool verbose)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5072,7 +5177,10 @@ listTSParsersVerbose(const char *pattern)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5215,7 +5323,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
@@ -5281,7 +5392,10 @@ listTSDictionaries(const char *pattern, bool verbose)
 								"n.nspname", "d.dictname", NULL,
 								"pg_catalog.pg_ts_dict_is_visible(d.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5344,7 +5458,10 @@ listTSTemplates(const char *pattern, bool verbose)
 								"n.nspname", "t.tmplname", NULL,
 								"pg_catalog.pg_ts_template_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5396,7 +5513,10 @@ listTSConfigs(const char *pattern, bool verbose)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5440,7 +5560,10 @@ listTSConfigsVerbose(const char *pattern)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 3, 2;");
 
@@ -5613,7 +5736,10 @@ listForeignDataWrappers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "fdwname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5687,7 +5813,10 @@ listForeignServers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "s.srvname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5740,7 +5869,10 @@ listUserMappings(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "um.srvname", "um.usename", NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5810,7 +5942,10 @@ listForeignTables(const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5859,7 +5994,10 @@ listExtensions(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5900,7 +6038,10 @@ listExtensionContents(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6023,15 +6164,18 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 		if (PQdb(pset.db) == NULL)
 		{
 			pg_log_error("You are currently not connected to a database.");
+			termPQExpBuffer(&dbbuf);
 			return false;
 		}
 		if (strcmp(PQdb(pset.db), dbbuf.data) != 0)
 		{
 			pg_log_error("cross-database references are not implemented: %s",
 						 pattern);
+			termPQExpBuffer(&dbbuf);
 			return false;
 		}
 	}
+	termPQExpBuffer(&dbbuf);
 	return true;
 }
 
@@ -6090,7 +6234,10 @@ listPublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6205,7 +6352,10 @@ describePublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2;");
 
@@ -6417,7 +6567,10 @@ describeSubscriptions(const char *pattern, bool verbose)
 								NULL, "subname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6521,7 +6674,9 @@ listOperatorClasses(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+		{
+			goto error_return;
+		}
 	if (type_pattern)
 	{
 		/* Match type name pattern against either internal or external name */
@@ -6530,7 +6685,9 @@ listOperatorClasses(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+		{
+			goto error_return;
+		}
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
@@ -6549,6 +6706,10 @@ listOperatorClasses(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6597,7 +6758,9 @@ listOperatorFamilies(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+		{
+			goto error_return;
+		}
 	if (type_pattern)
 	{
 		appendPQExpBuffer(&buf,
@@ -6614,7 +6777,9 @@ listOperatorFamilies(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+		{
+			goto error_return;
+		}
 		appendPQExpBufferStr(&buf, "  )\n");
 	}
 
@@ -6634,6 +6799,10 @@ listOperatorFamilies(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6696,13 +6865,17 @@ listOpFamilyOperators(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+		{
+			goto error_return;
+		}
 
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"nsf.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+		{
+			goto error_return;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  o.amoplefttype = o.amoprighttype DESC,\n"
@@ -6725,6 +6898,10 @@ listOpFamilyOperators(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6784,12 +6961,16 @@ listOpFamilyFunctions(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+		{
+			goto error_return;
+		}
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"ns.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+		{
+			goto error_return;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  ap.amproclefttype = ap.amprocrighttype DESC,\n"
@@ -6810,6 +6991,10 @@ listOpFamilyFunctions(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
-- 
2.36.1

#12

Japin Li

japinli@hotmail.com

over 3 years ago

In reply to: Michael Paquier (#11)

Re: Memory leak fix in psql

On Wed, 20 Jul 2022 at 11:51, Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Jul 20, 2022 at 03:14:35AM +0000, tanghy.fnst@fujitsu.com wrote:

Your fix LGTM so please allow me to merge it in the attached patch.
Based on your rebased version, now this new patch version is V3.

What about the argument of upthread where we could use a goto in
functions where there are multiple pattern validation checks? Per se
v4 attached.

+1. LGTM.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

#13

Junwang Zhao

zhjwpku@gmail.com

over 3 years ago

In reply to: Michael Paquier (#11)

Re: Memory leak fix in psql

-1

Though the patch looks good, I myself think the patch should be edited
and submitted by Tang
It's easy to attach a fixed patch based on the comments of the thread,
but coins should be
given to Tang since he is the first one to find the mem leak.

No offense, but that's what I think how open source works ;)

On Wed, Jul 20, 2022 at 11:51 AM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Jul 20, 2022 at 03:14:35AM +0000, tanghy.fnst@fujitsu.com wrote:

Your fix LGTM so please allow me to merge it in the attached patch.
Based on your rebased version, now this new patch version is V3.

What about the argument of upthread where we could use a goto in
functions where there are multiple pattern validation checks? Per se
v4 attached.
--
Michael

--
Regards
Junwang Zhao

#14

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: Junwang Zhao (#13)

Re: Memory leak fix in psql

On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:

Though the patch looks good, I myself think the patch should be edited
and submitted by Tang
It's easy to attach a fixed patch based on the comments of the thread,
but coins should be
given to Tang since he is the first one to find the mem leak.

Please note that I sometimes edit slightly patches that I finish to
merge into the tree, where the author listed in the commit log is the
same as the original while I usually don't list mine. Credit goes
where it should, and Tang is the one here who authored this patch.
--
Michael

#15

Junwang Zhao

zhjwpku@gmail.com

over 3 years ago

In reply to: Michael Paquier (#14)

Re: Memory leak fix in psql

Got it, thanks!

On Wed, Jul 20, 2022 at 1:14 PM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:

Though the patch looks good, I myself think the patch should be edited
and submitted by Tang
It's easy to attach a fixed patch based on the comments of the thread,
but coins should be
given to Tang since he is the first one to find the mem leak.

Please note that I sometimes edit slightly patches that I finish to
merge into the tree, where the author listed in the commit log is the
same as the original while I usually don't list mine. Credit goes
where it should, and Tang is the one here who authored this patch.
--
Michael

--
Regards
Junwang Zhao

#16

tanghy.fnst@fujitsu.com

over 3 years ago

In reply to: Michael Paquier (#11)

1 attachment(s)

RE: Memory leak fix in psql

On Wednesday, July 20, 2022 12:52 PM, Michael Paquier <michael@paquier.xyz> wrote:

What about the argument of upthread where we could use a goto in
functions where there are multiple pattern validation checks? Per se
v4 attached.

Thanks for your kindly remind and modification.
I checked v4 patch, it looks good but I think there can be some minor improvement.
So I deleted some redundant braces around "goto error_return; ".
Also added an error handle section in validateSQLNamePattern.

Kindly to have a check at the attached v5 patch.

Regards,
Tang

Attachments:

v5-0001-fix-the-memory-leak-in-psql-describe.patchapplication/octet-stream; name=v5-0001-fix-the-memory-leak-in-psql-describe.patchDownload

From 1dd828a40d5a45c0ee021f42f8eee354082a72cf Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 20 Jul 2022 12:47:52 +0900
Subject: [PATCH v5] fix the memory leak in psql describe


diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 88d92a08ae..77b0f87e39 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -112,7 +112,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
 
@@ -182,7 +185,10 @@ describeAccessMethods(const char *pattern, bool verbose)
 								NULL, "amname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -244,7 +250,10 @@ describeTablespaces(const char *pattern, bool verbose)
 								NULL, "spcname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -534,7 +543,7 @@ describeFunctions(const char *functypes, const char *func_pattern,
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	for (int i = 0; i < num_arg_patterns; i++)
 	{
@@ -561,7 +570,7 @@ describeFunctions(const char *functypes, const char *func_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+				goto error_return;
 		}
 		else
 		{
@@ -599,6 +608,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -682,7 +695,10 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
 								"pg_catalog.format_type(t.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -836,7 +852,7 @@ describeOperators(const char *oper_pattern,
 								"n.nspname", "o.oprname", NULL,
 								"pg_catalog.pg_operator_is_visible(o.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	if (num_arg_patterns == 1)
 		appendPQExpBufferStr(&buf, "  AND o.oprleft = 0\n");
@@ -866,7 +882,7 @@ describeOperators(const char *oper_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+				goto error_return;
 		}
 		else
 		{
@@ -890,6 +906,10 @@ describeOperators(const char *oper_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -953,7 +973,10 @@ listAllDbs(const char *pattern, bool verbose)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "d.datname", NULL, NULL,
 									NULL, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 	res = PSQLexec(buf.data);
@@ -1106,7 +1129,10 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -1177,16 +1203,13 @@ listDefaultACLs(const char *pattern)
 								"pg_catalog.pg_get_userbyid(d.defaclrole)",
 								NULL,
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 3;");
 
 	res = PSQLexec(buf.data);
 	if (!res)
-	{
-		termPQExpBuffer(&buf);
-		return false;
-	}
+		goto error_return;
 
 	myopt.nullPrint = NULL;
 	printfPQExpBuffer(&buf, _("Default access privileges"));
@@ -1200,6 +1223,10 @@ listDefaultACLs(const char *pattern)
 	termPQExpBuffer(&buf);
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1253,7 +1280,7 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Domain constraint descriptions */
 	appendPQExpBuffer(&buf,
@@ -1277,7 +1304,7 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Operator class descriptions */
 	appendPQExpBuffer(&buf,
@@ -1301,7 +1328,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "o.opcname", NULL,
 								"pg_catalog.pg_opclass_is_visible(o.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Operator family descriptions */
 	appendPQExpBuffer(&buf,
@@ -1325,7 +1352,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "opf.opfname", NULL,
 								"pg_catalog.pg_opfamily_is_visible(opf.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Rule descriptions (ignore rules for views) */
 	appendPQExpBuffer(&buf,
@@ -1348,7 +1375,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "r.rulename", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Trigger descriptions */
 	appendPQExpBuffer(&buf,
@@ -1370,7 +1397,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "t.tgname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf,
 						 ") AS tt\n"
@@ -1393,6 +1420,10 @@ objectDescription(const char *pattern, bool showSystem)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1428,7 +1459,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2, 3;");
 
@@ -3614,7 +3648,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -3739,11 +3776,11 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 					  gettext_noop("Settings"));
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL, &havewhere, 1))
-		return false;
+		goto error_return;
 	if (!validateSQLNamePattern(&buf, pattern2, havewhere, false,
 								NULL, "d.datname", NULL, NULL,
 								NULL, 1))
-		return false;
+		goto error_return;
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
@@ -3779,6 +3816,10 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -3940,7 +3981,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
 
@@ -4157,7 +4201,10 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
 					  mixed_output ? "\"Type\" DESC, " : "",
@@ -4233,7 +4280,10 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "l.lanname", NULL, NULL,
 									NULL, 2))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf, "WHERE l.lanplcallfoid != 0\n");
@@ -4319,7 +4369,10 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "t.typname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4395,7 +4448,10 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.conname", NULL,
 								"pg_catalog.pg_conversion_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4542,7 +4598,10 @@ listEventTriggers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "evtname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1");
 
@@ -4638,7 +4697,10 @@ listExtendedStats(const char *pattern)
 								"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text", "es.stxname",
 								NULL, "pg_catalog.pg_statistics_obj_is_visible(es.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4742,7 +4804,7 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(ts.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(ts.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, ") OR (true");
 
@@ -4751,7 +4813,7 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(tt.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(tt.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, ") )\nORDER BY 1, 2;");
 
@@ -4770,6 +4832,10 @@ listCasts(const char *pattern, bool verbose)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -4851,7 +4917,10 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.collname", NULL,
 								"pg_catalog.pg_collation_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4914,7 +4983,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5029,7 +5101,10 @@ listTSParsers(const char *pattern, bool verbose)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5072,7 +5147,10 @@ listTSParsersVerbose(const char *pattern)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5215,7 +5293,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
@@ -5281,7 +5362,10 @@ listTSDictionaries(const char *pattern, bool verbose)
 								"n.nspname", "d.dictname", NULL,
 								"pg_catalog.pg_ts_dict_is_visible(d.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5344,7 +5428,10 @@ listTSTemplates(const char *pattern, bool verbose)
 								"n.nspname", "t.tmplname", NULL,
 								"pg_catalog.pg_ts_template_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5396,7 +5483,10 @@ listTSConfigs(const char *pattern, bool verbose)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5440,7 +5530,10 @@ listTSConfigsVerbose(const char *pattern)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 3, 2;");
 
@@ -5613,7 +5706,10 @@ listForeignDataWrappers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "fdwname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5687,7 +5783,10 @@ listForeignServers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "s.srvname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5740,7 +5839,10 @@ listUserMappings(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "um.srvname", "um.usename", NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5810,7 +5912,10 @@ listForeignTables(const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5859,7 +5964,10 @@ listExtensions(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5900,7 +6008,10 @@ listExtensionContents(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6014,8 +6125,7 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 	{
 		pg_log_error("improper qualified name (too many dotted names): %s",
 					 pattern);
-		termPQExpBuffer(&dbbuf);
-		return false;
+		goto error_return;
 	}
 
 	if (maxparts > 1 && dotcnt == maxparts - 1)
@@ -6023,16 +6133,21 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 		if (PQdb(pset.db) == NULL)
 		{
 			pg_log_error("You are currently not connected to a database.");
-			return false;
+			goto error_return;
 		}
 		if (strcmp(PQdb(pset.db), dbbuf.data) != 0)
 		{
 			pg_log_error("cross-database references are not implemented: %s",
 						 pattern);
-			return false;
+			goto error_return;
 		}
 	}
+	termPQExpBuffer(&dbbuf);
 	return true;
+
+error_return:
+	termPQExpBuffer(&dbbuf);
+	return false;
 }
 
 /*
@@ -6090,7 +6205,10 @@ listPublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6205,7 +6323,10 @@ describePublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2;");
 
@@ -6417,7 +6538,10 @@ describeSubscriptions(const char *pattern, bool verbose)
 								NULL, "subname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6521,7 +6645,7 @@ listOperatorClasses(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (type_pattern)
 	{
 		/* Match type name pattern against either internal or external name */
@@ -6530,7 +6654,7 @@ listOperatorClasses(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
@@ -6549,6 +6673,10 @@ listOperatorClasses(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6597,7 +6725,7 @@ listOperatorFamilies(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (type_pattern)
 	{
 		appendPQExpBuffer(&buf,
@@ -6614,7 +6742,7 @@ listOperatorFamilies(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 		appendPQExpBufferStr(&buf, "  )\n");
 	}
 
@@ -6634,6 +6762,10 @@ listOperatorFamilies(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6696,13 +6828,13 @@ listOpFamilyOperators(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"nsf.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+			goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  o.amoplefttype = o.amoprighttype DESC,\n"
@@ -6725,6 +6857,10 @@ listOpFamilyOperators(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6784,12 +6920,12 @@ listOpFamilyFunctions(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"ns.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+			goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  ap.amproclefttype = ap.amprocrighttype DESC,\n"
@@ -6810,6 +6946,10 @@ listOpFamilyFunctions(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
-- 
2.33.0.windows.2

#17

tanghy.fnst@fujitsu.com

over 3 years ago

In reply to: Junwang Zhao (#15)

RE: Memory leak fix in psql

On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:

Though the patch looks good, I myself think the patch should be edited
and submitted by Tang
It's easy to attach a fixed patch based on the comments of the thread,
but coins should be
given to Tang since he is the first one to find the mem leak.

Hello, Zhao

Thanks for your check at this patch.

I appreciate your kindly comment but there may be a misunderstanding here.
As Michael explained, committers in Postgres will review carefully and
help to improve contributors' patches. When the patch is finally committed
by one committer, from what I can see, he or she will try to make sure the
credit goes with everyone who contributed to the committed patch(such as
bug reporter, patch author, tester, reviewer etc.).

Also, developers and reviewers will try to help improving our proposed patch
by rebasing it or adding an on-top patch(like Japin Li did in v2).
These will make the patch better and to be committed ASAP.

Good to see you at Postgres community.

Regards,
Tang

#18

Junwang Zhao

zhjwpku@gmail.com

over 3 years ago

In reply to: tanghy.fnst@fujitsu.com (#17)

Re: Memory leak fix in psql

Thanks for your explanation, this time I know how it works, thanks ;)

On Wed, Jul 20, 2022 at 3:04 PM tanghy.fnst@fujitsu.com
<tanghy.fnst@fujitsu.com> wrote:

On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:

Though the patch looks good, I myself think the patch should be edited
and submitted by Tang
It's easy to attach a fixed patch based on the comments of the thread,
but coins should be
given to Tang since he is the first one to find the mem leak.

Hello, Zhao

Thanks for your check at this patch.

I appreciate your kindly comment but there may be a misunderstanding here.
As Michael explained, committers in Postgres will review carefully and
help to improve contributors' patches. When the patch is finally committed
by one committer, from what I can see, he or she will try to make sure the
credit goes with everyone who contributed to the committed patch(such as
bug reporter, patch author, tester, reviewer etc.).

Also, developers and reviewers will try to help improving our proposed patch
by rebasing it or adding an on-top patch(like Japin Li did in v2).
These will make the patch better and to be committed ASAP.

Good to see you at Postgres community.

Regards,
Tang

--
Regards
Junwang Zhao

#19

Alvaro Herrera

alvherre@alvh.no-ip.org

over 3 years ago

In reply to: Michael Paquier (#11)

1 attachment(s)

Re: Memory leak fix in psql

Attachments:

0001-More-goto-error_return-in-describe.c.patchtext/x-diff; charset=us-asciiDownload

From 5e031fadc778365491f9e58da83a4b64f21e3bcd Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 20 Jul 2022 10:04:27 +0200
Subject: [PATCH] More goto error_return in describe.c

---
 src/bin/psql/describe.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 77b0f87e39..46999f26f2 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1129,19 +1129,13 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-	{
-		termPQExpBuffer(&buf);
-		return false;
-	}
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
 	if (!res)
-	{
-		termPQExpBuffer(&buf);
-		return false;
-	}
+		goto error_return;
 
 	myopt.nullPrint = NULL;
 	printfPQExpBuffer(&buf, _("Access privileges"));
@@ -1155,6 +1149,10 @@ permissionsList(const char *pattern)
 	termPQExpBuffer(&buf);
 	PQclear(res);
 	return true;
+
+error_return:
+		termPQExpBuffer(&buf);
+		return false;
 }
 
 
@@ -4983,19 +4981,13 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
-	{
-		termPQExpBuffer(&buf);
-		return false;
-	}
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
 	res = PSQLexec(buf.data);
 	if (!res)
-	{
-		termPQExpBuffer(&buf);
-		return false;
-	}
+		goto error_return;
 
 	myopt.nullPrint = NULL;
 	myopt.title = _("List of schemas");
@@ -5016,10 +5008,7 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 						  pattern);
 		result = PSQLexec(buf.data);
 		if (!result)
-		{
-			termPQExpBuffer(&buf);
-			return false;
-		}
+			goto error_return;
 		else
 			pub_schema_tuples = PQntuples(result);
 
@@ -5066,6 +5055,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 	}
 
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
-- 
2.30.2

#20

Japin Li

japinli@hotmail.com

over 3 years ago

In reply to: tanghy.fnst@fujitsu.com (#16)

3 attachment(s)

Re: Memory leak fix in psql

On Wed, 20 Jul 2022 at 14:21, tanghy.fnst@fujitsu.com <tanghy.fnst@fujitsu.com> wrote:

On Wednesday, July 20, 2022 12:52 PM, Michael Paquier <michael@paquier.xyz> wrote:

What about the argument of upthread where we could use a goto in
functions where there are multiple pattern validation checks? Per se
v4 attached.

Thanks for your kindly remind and modification.
I checked v4 patch, it looks good but I think there can be some minor improvement.
So I deleted some redundant braces around "goto error_return; ".
Also added an error handle section in validateSQLNamePattern.

Kindly to have a check at the attached v5 patch.

Regards,
Tang

Thanks for updating the patch. It looks good. However, it cannot be
applied on 14 stable. The attached patches are for 10-14.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Attachments:

pg14-v5-0001-fix-the-memory-leak-in-psql-describe.patchtext/x-patchDownload

From fee69e1f3b56d4639aed789ee38850d68fb052a6 Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Wed, 20 Jul 2022 15:52:45 +0800
Subject: [PATCH v5 1/1] fix the memory leak in psql describe

---
 src/bin/psql/describe.c | 201 ++++++++++++++++++++++++++++++++++------
 1 file changed, 172 insertions(+), 29 deletions(-)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 8fe39475fa..c6ff5a9d14 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -131,7 +131,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
 							  "n.nspname", "p.proname", NULL,
 							  "pg_catalog.pg_function_is_visible(p.oid)",
 							  NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
 
@@ -201,7 +204,10 @@ describeAccessMethods(const char *pattern, bool verbose)
 								NULL, "amname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -290,7 +296,10 @@ describeTablespaces(const char *pattern, bool verbose)
 								NULL, "spcname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -656,7 +665,7 @@ describeFunctions(const char *functypes, const char *func_pattern,
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	for (int i = 0; i < num_arg_patterns; i++)
 	{
@@ -683,7 +692,7 @@ describeFunctions(const char *functypes, const char *func_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+				goto error_return;
 		}
 		else
 		{
@@ -721,6 +730,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -827,7 +840,10 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
 								"pg_catalog.format_type(t.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -982,7 +998,7 @@ describeOperators(const char *oper_pattern,
 								"n.nspname", "o.oprname", NULL,
 								"pg_catalog.pg_operator_is_visible(o.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	if (num_arg_patterns == 1)
 		appendPQExpBufferStr(&buf, "  AND o.oprleft = 0\n");
@@ -1012,7 +1028,7 @@ describeOperators(const char *oper_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+				goto error_return;
 		}
 		else
 		{
@@ -1036,6 +1052,10 @@ describeOperators(const char *oper_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1093,7 +1113,10 @@ listAllDbs(const char *pattern, bool verbose)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "d.datname", NULL, NULL,
 									NULL, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 	res = PSQLexec(buf.data);
@@ -1247,7 +1270,10 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -1328,7 +1354,10 @@ listDefaultACLs(const char *pattern)
 								"pg_catalog.pg_get_userbyid(d.defaclrole)",
 								NULL,
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 3;");
 
@@ -1404,7 +1433,7 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Domain constraint descriptions */
 	appendPQExpBuffer(&buf,
@@ -1428,7 +1457,7 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 
 	/*
@@ -1458,7 +1487,7 @@ objectDescription(const char *pattern, bool showSystem)
 									"n.nspname", "o.opcname", NULL,
 									"pg_catalog.pg_opclass_is_visible(o.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 	}
 
 	/*
@@ -1489,7 +1518,7 @@ objectDescription(const char *pattern, bool showSystem)
 									"n.nspname", "opf.opfname", NULL,
 									"pg_catalog.pg_opfamily_is_visible(opf.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 	}
 
 	/* Rule descriptions (ignore rules for views) */
@@ -1513,7 +1542,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "r.rulename", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Trigger descriptions */
 	appendPQExpBuffer(&buf,
@@ -1535,7 +1564,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "t.tgname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf,
 						 ") AS tt\n"
@@ -1558,6 +1587,10 @@ objectDescription(const char *pattern, bool showSystem)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1593,7 +1626,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2, 3;");
 
@@ -3852,7 +3888,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "r.rolname", NULL, NULL,
 									NULL, 1))
-			return false;
+			goto error_return;
 	}
 	else
 	{
@@ -3869,14 +3905,14 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "u.usename", NULL, NULL,
 									NULL, 1))
-			return false;
+			goto error_return;
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
 	res = PSQLexec(buf.data);
 	if (!res)
-		return false;
+		goto error_return;
 
 	nrows = PQntuples(res);
 	attr = pg_malloc0((nrows + 1) * sizeof(*attr));
@@ -3962,6 +3998,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 static void
@@ -4006,11 +4046,11 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 					  gettext_noop("Settings"));
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL, &havewhere, 1))
-		return false;
+		goto error_return;
 	if (!validateSQLNamePattern(&buf, pattern2, havewhere, false,
 								NULL, "d.datname", NULL, NULL,
 								NULL, 1))
-		return false;
+		goto error_return;
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
@@ -4046,6 +4086,10 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -4229,7 +4273,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
 
@@ -4446,7 +4493,10 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
 					  mixed_output ? "\"Type\" DESC, " : "",
@@ -4527,7 +4577,10 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "l.lanname", NULL, NULL,
 									NULL, 2))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf, "WHERE l.lanplcallfoid != 0\n");
@@ -4620,7 +4673,10 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "t.typname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4696,7 +4752,10 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.conname", NULL,
 								"pg_catalog.pg_conversion_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4764,7 +4823,10 @@ listEventTriggers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "evtname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1");
 
@@ -4860,7 +4922,10 @@ listExtendedStats(const char *pattern)
 								"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text", "es.stxname",
 								NULL, "pg_catalog.pg_statistics_obj_is_visible(es.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4971,7 +5036,7 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(ts.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(ts.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, ") OR (true");
 
@@ -4980,7 +5045,7 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(tt.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(tt.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, ") )\nORDER BY 1, 2;");
 
@@ -4999,6 +5064,10 @@ listCasts(const char *pattern, bool verbose)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -5081,7 +5150,10 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.collname", NULL,
 								"pg_catalog.pg_collation_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5142,7 +5214,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5204,7 +5279,10 @@ listTSParsers(const char *pattern, bool verbose)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5247,7 +5325,10 @@ listTSParsersVerbose(const char *pattern)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5390,7 +5471,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
@@ -5466,7 +5550,10 @@ listTSDictionaries(const char *pattern, bool verbose)
 								"n.nspname", "d.dictname", NULL,
 								"pg_catalog.pg_ts_dict_is_visible(d.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5539,7 +5626,10 @@ listTSTemplates(const char *pattern, bool verbose)
 								"n.nspname", "t.tmplname", NULL,
 								"pg_catalog.pg_ts_template_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5601,7 +5691,10 @@ listTSConfigs(const char *pattern, bool verbose)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5645,7 +5738,10 @@ listTSConfigsVerbose(const char *pattern)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 3, 2;");
 
@@ -5834,7 +5930,10 @@ listForeignDataWrappers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "fdwname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5918,7 +6017,10 @@ listForeignServers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "s.srvname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5981,7 +6083,10 @@ listUserMappings(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "um.srvname", "um.usename", NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -6061,7 +6166,10 @@ listForeignTables(const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -6120,7 +6228,10 @@ listExtensions(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6171,7 +6282,10 @@ listExtensionContents(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6285,8 +6399,7 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 	{
 		pg_log_error("improper qualified name (too many dotted names): %s",
 					 pattern);
-		termPQExpBuffer(&dbbuf);
-		return false;
+		goto error_return;
 	}
 
 	if (maxparts > 1 && dotcnt == maxparts-1)
@@ -6294,16 +6407,21 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 		if (PQdb(pset.db) == NULL)
 		{
 			pg_log_error("You are currently not connected to a database.");
-			return false;
+			goto error_return;
 		}
 		if (strcmp(PQdb(pset.db), dbbuf.data) != 0)
 		{
 			pg_log_error("cross-database references are not implemented: %s",
 						 pattern);
-			return false;
+			goto error_return;
 		}
 	}
+	termPQExpBuffer(&dbbuf);
 	return true;
+
+error_return:
+	termPQExpBuffer(&dbbuf);
+	return false;
 }
 
 /*
@@ -6361,7 +6479,10 @@ listPublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6430,7 +6551,10 @@ describePublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2;");
 
@@ -6616,7 +6740,10 @@ describeSubscriptions(const char *pattern, bool verbose)
 								NULL, "subname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6725,7 +6852,7 @@ listOperatorClasses(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (type_pattern)
 	{
 		/* Match type name pattern against either internal or external name */
@@ -6734,7 +6861,7 @@ listOperatorClasses(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
@@ -6753,6 +6880,10 @@ listOperatorClasses(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+ error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6801,7 +6932,7 @@ listOperatorFamilies(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (type_pattern)
 	{
 		appendPQExpBuffer(&buf,
@@ -6818,7 +6949,7 @@ listOperatorFamilies(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 		appendPQExpBufferStr(&buf, "  )\n");
 	}
 
@@ -6838,6 +6969,10 @@ listOperatorFamilies(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6900,13 +7035,13 @@ listOpFamilyOperators(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"nsf.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+			goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  o.amoplefttype = o.amoprighttype DESC,\n"
@@ -6929,6 +7064,10 @@ listOpFamilyOperators(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6988,12 +7127,12 @@ listOpFamilyFunctions(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"ns.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+			goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  ap.amproclefttype = ap.amprocrighttype DESC,\n"
@@ -7014,4 +7153,8 @@ listOpFamilyFunctions(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
-- 
2.17.1

v5-0001-fix-the-memory-leak-in-psql-describe.patchtext/x-patchDownload

From 1dd828a40d5a45c0ee021f42f8eee354082a72cf Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 20 Jul 2022 12:47:52 +0900
Subject: [PATCH v5] fix the memory leak in psql describe


diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 88d92a08ae..77b0f87e39 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -112,7 +112,10 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
 
@@ -182,7 +185,10 @@ describeAccessMethods(const char *pattern, bool verbose)
 								NULL, "amname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -244,7 +250,10 @@ describeTablespaces(const char *pattern, bool verbose)
 								NULL, "spcname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -534,7 +543,7 @@ describeFunctions(const char *functypes, const char *func_pattern,
 								"n.nspname", "p.proname", NULL,
 								"pg_catalog.pg_function_is_visible(p.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	for (int i = 0; i < num_arg_patterns; i++)
 	{
@@ -561,7 +570,7 @@ describeFunctions(const char *functypes, const char *func_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+				goto error_return;
 		}
 		else
 		{
@@ -599,6 +608,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -682,7 +695,10 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
 								"pg_catalog.format_type(t.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -836,7 +852,7 @@ describeOperators(const char *oper_pattern,
 								"n.nspname", "o.oprname", NULL,
 								"pg_catalog.pg_operator_is_visible(o.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	if (num_arg_patterns == 1)
 		appendPQExpBufferStr(&buf, "  AND o.oprleft = 0\n");
@@ -866,7 +882,7 @@ describeOperators(const char *oper_pattern,
 										true, false,
 										nspname, typname, ft, tiv,
 										NULL, 3))
-				return false;
+				goto error_return;
 		}
 		else
 		{
@@ -890,6 +906,10 @@ describeOperators(const char *oper_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -953,7 +973,10 @@ listAllDbs(const char *pattern, bool verbose)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "d.datname", NULL, NULL,
 									NULL, 1))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 	res = PSQLexec(buf.data);
@@ -1106,7 +1129,10 @@ permissionsList(const char *pattern)
 								"n.nspname", "c.relname", NULL,
 								"n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -1177,16 +1203,13 @@ listDefaultACLs(const char *pattern)
 								"pg_catalog.pg_get_userbyid(d.defaclrole)",
 								NULL,
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 3;");
 
 	res = PSQLexec(buf.data);
 	if (!res)
-	{
-		termPQExpBuffer(&buf);
-		return false;
-	}
+		goto error_return;
 
 	myopt.nullPrint = NULL;
 	printfPQExpBuffer(&buf, _("Default access privileges"));
@@ -1200,6 +1223,10 @@ listDefaultACLs(const char *pattern)
 	termPQExpBuffer(&buf);
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1253,7 +1280,7 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Domain constraint descriptions */
 	appendPQExpBuffer(&buf,
@@ -1277,7 +1304,7 @@ objectDescription(const char *pattern, bool showSystem)
 								false, "n.nspname", "pgc.conname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Operator class descriptions */
 	appendPQExpBuffer(&buf,
@@ -1301,7 +1328,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "o.opcname", NULL,
 								"pg_catalog.pg_opclass_is_visible(o.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Operator family descriptions */
 	appendPQExpBuffer(&buf,
@@ -1325,7 +1352,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "opf.opfname", NULL,
 								"pg_catalog.pg_opfamily_is_visible(opf.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Rule descriptions (ignore rules for views) */
 	appendPQExpBuffer(&buf,
@@ -1348,7 +1375,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "r.rulename", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	/* Trigger descriptions */
 	appendPQExpBuffer(&buf,
@@ -1370,7 +1397,7 @@ objectDescription(const char *pattern, bool showSystem)
 								"n.nspname", "t.tgname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf,
 						 ") AS tt\n"
@@ -1393,6 +1420,10 @@ objectDescription(const char *pattern, bool showSystem)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -1428,7 +1459,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2, 3;");
 
@@ -3614,7 +3648,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -3739,11 +3776,11 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 					  gettext_noop("Settings"));
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "r.rolname", NULL, NULL, &havewhere, 1))
-		return false;
+		goto error_return;
 	if (!validateSQLNamePattern(&buf, pattern2, havewhere, false,
 								NULL, "d.datname", NULL, NULL,
 								NULL, 1))
-		return false;
+		goto error_return;
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
 	res = PSQLexec(buf.data);
@@ -3779,6 +3816,10 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 
@@ -3940,7 +3981,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
 
@@ -4157,7 +4201,10 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
 					  mixed_output ? "\"Type\" DESC, " : "",
@@ -4233,7 +4280,10 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
 		if (!validateSQLNamePattern(&buf, pattern, false, false,
 									NULL, "l.lanname", NULL, NULL,
 									NULL, 2))
+		{
+			termPQExpBuffer(&buf);
 			return false;
+		}
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf, "WHERE l.lanplcallfoid != 0\n");
@@ -4319,7 +4369,10 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "t.typname", NULL,
 								"pg_catalog.pg_type_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4395,7 +4448,10 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.conname", NULL,
 								"pg_catalog.pg_conversion_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4542,7 +4598,10 @@ listEventTriggers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "evtname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1");
 
@@ -4638,7 +4697,10 @@ listExtendedStats(const char *pattern)
 								"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text", "es.stxname",
 								NULL, "pg_catalog.pg_statistics_obj_is_visible(es.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4742,7 +4804,7 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(ts.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(ts.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, ") OR (true");
 
@@ -4751,7 +4813,7 @@ listCasts(const char *pattern, bool verbose)
 								"pg_catalog.format_type(tt.oid, NULL)",
 								"pg_catalog.pg_type_is_visible(tt.oid)",
 								NULL, 3))
-		return false;
+		goto error_return;
 
 	appendPQExpBufferStr(&buf, ") )\nORDER BY 1, 2;");
 
@@ -4770,6 +4832,10 @@ listCasts(const char *pattern, bool verbose)
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -4851,7 +4917,10 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
 								"n.nspname", "c.collname", NULL,
 								"pg_catalog.pg_collation_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -4914,7 +4983,10 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 								NULL, "n.nspname", NULL,
 								NULL,
 								NULL, 2))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5029,7 +5101,10 @@ listTSParsers(const char *pattern, bool verbose)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5072,7 +5147,10 @@ listTSParsersVerbose(const char *pattern)
 								"n.nspname", "p.prsname", NULL,
 								"pg_catalog.pg_ts_parser_is_visible(p.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5215,7 +5293,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
@@ -5281,7 +5362,10 @@ listTSDictionaries(const char *pattern, bool verbose)
 								"n.nspname", "d.dictname", NULL,
 								"pg_catalog.pg_ts_dict_is_visible(d.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5344,7 +5428,10 @@ listTSTemplates(const char *pattern, bool verbose)
 								"n.nspname", "t.tmplname", NULL,
 								"pg_catalog.pg_ts_template_is_visible(t.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5396,7 +5483,10 @@ listTSConfigs(const char *pattern, bool verbose)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5440,7 +5530,10 @@ listTSConfigsVerbose(const char *pattern)
 								"n.nspname", "c.cfgname", NULL,
 								"pg_catalog.pg_ts_config_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 3, 2;");
 
@@ -5613,7 +5706,10 @@ listForeignDataWrappers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "fdwname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5687,7 +5783,10 @@ listForeignServers(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "s.srvname", NULL, NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5740,7 +5839,10 @@ listUserMappings(const char *pattern, bool verbose)
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "um.srvname", "um.usename", NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5810,7 +5912,10 @@ listForeignTables(const char *pattern, bool verbose)
 								"n.nspname", "c.relname", NULL,
 								"pg_catalog.pg_table_is_visible(c.oid)",
 								NULL, 3))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2;");
 
@@ -5859,7 +5964,10 @@ listExtensions(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -5900,7 +6008,10 @@ listExtensionContents(const char *pattern)
 								NULL, "e.extname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6014,8 +6125,7 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 	{
 		pg_log_error("improper qualified name (too many dotted names): %s",
 					 pattern);
-		termPQExpBuffer(&dbbuf);
-		return false;
+		goto error_return;
 	}
 
 	if (maxparts > 1 && dotcnt == maxparts - 1)
@@ -6023,16 +6133,21 @@ validateSQLNamePattern(PQExpBuffer buf, const char *pattern, bool have_where,
 		if (PQdb(pset.db) == NULL)
 		{
 			pg_log_error("You are currently not connected to a database.");
-			return false;
+			goto error_return;
 		}
 		if (strcmp(PQdb(pset.db), dbbuf.data) != 0)
 		{
 			pg_log_error("cross-database references are not implemented: %s",
 						 pattern);
-			return false;
+			goto error_return;
 		}
 	}
+	termPQExpBuffer(&dbbuf);
 	return true;
+
+error_return:
+	termPQExpBuffer(&dbbuf);
+	return false;
 }
 
 /*
@@ -6090,7 +6205,10 @@ listPublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6205,7 +6323,10 @@ describePublications(const char *pattern)
 								NULL, "pubname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 2;");
 
@@ -6417,7 +6538,10 @@ describeSubscriptions(const char *pattern, bool verbose)
 								NULL, "subname", NULL,
 								NULL,
 								NULL, 1))
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 
@@ -6521,7 +6645,7 @@ listOperatorClasses(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (type_pattern)
 	{
 		/* Match type name pattern against either internal or external name */
@@ -6530,7 +6654,7 @@ listOperatorClasses(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 	}
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2, 4;");
@@ -6549,6 +6673,10 @@ listOperatorClasses(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6597,7 +6725,7 @@ listOperatorFamilies(const char *access_method_pattern,
 		if (!validateSQLNamePattern(&buf, access_method_pattern,
 									false, false, NULL, "am.amname", NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (type_pattern)
 	{
 		appendPQExpBuffer(&buf,
@@ -6614,7 +6742,7 @@ listOperatorFamilies(const char *access_method_pattern,
 									"pg_catalog.format_type(t.oid, NULL)",
 									"pg_catalog.pg_type_is_visible(t.oid)",
 									NULL, 3))
-			return false;
+			goto error_return;
 		appendPQExpBufferStr(&buf, "  )\n");
 	}
 
@@ -6634,6 +6762,10 @@ listOperatorFamilies(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6696,13 +6828,13 @@ listOpFamilyOperators(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"nsf.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+			goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  o.amoplefttype = o.amoprighttype DESC,\n"
@@ -6725,6 +6857,10 @@ listOpFamilyOperators(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
@@ -6784,12 +6920,12 @@ listOpFamilyFunctions(const char *access_method_pattern,
 									false, false, NULL, "am.amname",
 									NULL, NULL,
 									&have_where, 1))
-			return false;
+			goto error_return;
 	if (family_pattern)
 		if (!validateSQLNamePattern(&buf, family_pattern, have_where, false,
 									"ns.nspname", "of.opfname", NULL, NULL,
 									NULL, 3))
-			return false;
+			goto error_return;
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1, 2,\n"
 						 "  ap.amproclefttype = ap.amprocrighttype DESC,\n"
@@ -6810,6 +6946,10 @@ listOpFamilyFunctions(const char *access_method_pattern,
 
 	PQclear(res);
 	return true;
+
+error_return:
+	termPQExpBuffer(&buf);
+	return false;
 }
 
 /*
-- 
2.33.0.windows.2

pg10-11-12-v5-0001-fix-the-memory-leak-in-psql-describe.patchtext/x-patchDownload

From 11b11f08f56ba77707e2a160cc36648b6f14effb Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Wed, 20 Jul 2022 14:12:53 +0800
Subject: [PATCH v5 1/1] fix the memory leak in psql describe

---
 src/bin/psql/describe.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c7ff55fbd7..32a18becba 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3441,7 +3441,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 
 	res = PSQLexec(buf.data);
 	if (!res)
+	{
+		termPQExpBuffer(&buf);
 		return false;
+	}
 
 	nrows = PQntuples(res);
 	attr = pg_malloc0((nrows + 1) * sizeof(*attr));
@@ -4800,7 +4803,10 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
 	res = PSQLexec(buf.data);
 	termPQExpBuffer(&buf);
 	if (!res)
+	{
+		termPQExpBuffer(&title);
 		return false;
+	}
 
 	myopt.nullPrint = NULL;
 	if (nspname)
-- 
2.17.1

#21

Tom Lane

tgl@sss.pgh.pa.us

over 3 years ago

In reply to: Japin Li (#20)

Re: Memory leak fix in psql

Japin Li <japinli@hotmail.com> writes:

Thanks for updating the patch. It looks good. However, it cannot be
applied on 14 stable. The attached patches are for 10-14.

While I think this is good cleanup, I'm doubtful that any of these
leaks are probable enough to be worth back-patching into stable
branches. The risk of breaking something should not be neglected.

regards, tom lane

#22

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: Alvaro Herrera (#19)

Re: Memory leak fix in psql

On Wed, Jul 20, 2022 at 10:05:47AM +0200, Alvaro Herrera wrote:

More on the same tune.

Thanks. I have noticed that as well. I'll include all that in the
set.
--
Michael

#23

Japin Li

japinli@hotmail.com

over 3 years ago

In reply to: Tom Lane (#21)

Re: Memory leak fix in psql

On Wed, 20 Jul 2022 at 21:54, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Japin Li <japinli@hotmail.com> writes:

Thanks for updating the patch. It looks good. However, it cannot be
applied on 14 stable. The attached patches are for 10-14.

While I think this is good cleanup, I'm doubtful that any of these
leaks are probable enough to be worth back-patching into stable
branches. The risk of breaking something should not be neglected.

Yeah, we should take care of the backpatch risk. However, I think
it makes sense to backpatch.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

#24

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: Japin Li (#23)

Re: Memory leak fix in psql

On Thu, Jul 21, 2022 at 09:10:43AM +0800, Japin Li wrote:

Yeah, we should take care of the backpatch risk. However, I think
it makes sense to backpatch.

We are talking about 256 bytes being leaked in each loop when a
validation pattern or when a query fails, so I don't see a strong
argument in manipulating 10~14 more than necessary for this amount of
memory. The contents of describe.c are the same for v15 though, and
we are still in beta on REL_15_STABLE, so I have applied the patch
down to v15, adding what Alvaro has sent on top of the rest.
--
Michael

#25

Japin Li

japinli@hotmail.com

over 3 years ago

In reply to: Michael Paquier (#24)

Re: Memory leak fix in psql

On Thu, 21 Jul 2022 at 09:48, Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jul 21, 2022 at 09:10:43AM +0800, Japin Li wrote:

Yeah, we should take care of the backpatch risk. However, I think
it makes sense to backpatch.

We are talking about 256 bytes being leaked in each loop when a
validation pattern or when a query fails, so I don't see a strong
argument in manipulating 10~14 more than necessary for this amount of
memory. The contents of describe.c are the same for v15 though, and
we are still in beta on REL_15_STABLE, so I have applied the patch
down to v15, adding what Alvaro has sent on top of the rest.

Thanks for the explanation! IMO, we could ignore v10-13 branches, however,
we should backpatch to v14 which also uses the validateSQLNamePattern()
function leading to a memory leak.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

#26

Alvaro Herrera

alvherre@alvh.no-ip.org

over 3 years ago

In reply to: Japin Li (#25)

Re: Memory leak fix in psql

On 2022-Jul-21, Japin Li wrote:

On Thu, 21 Jul 2022 at 09:48, Michael Paquier <michael@paquier.xyz> wrote:

We are talking about 256 bytes being leaked in each loop when a
validation pattern or when a query fails, so I don't see a strong
argument in manipulating 10~14 more than necessary for this amount of
memory. The contents of describe.c are the same for v15 though, and
we are still in beta on REL_15_STABLE, so I have applied the patch
down to v15, adding what Alvaro has sent on top of the rest.

Thanks for the explanation! IMO, we could ignore v10-13 branches, however,
we should backpatch to v14 which also uses the validateSQLNamePattern()
function leading to a memory leak.

I'd agree in principle, but in practice the commit from 15 does not apply
cleanly on 14 -- there's a ton of conflicts.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Here's a general engineering tip: if the non-fun part is too complex for you
to figure out, that might indicate the fun part is too ambitious." (John Naylor)
/messages/by-id/CAFBsxsG4OWHBbSDM=sSeXrQGOtkPiOEOuME4yD7Ce41NtaAD9g@mail.gmail.com

#27

Japin Li

japinli@hotmail.com

over 3 years ago

In reply to: Alvaro Herrera (#26)

Re: Memory leak fix in psql

On Thu, 21 Jul 2022 at 14:23, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

On 2022-Jul-21, Japin Li wrote:

On Thu, 21 Jul 2022 at 09:48, Michael Paquier <michael@paquier.xyz> wrote:

We are talking about 256 bytes being leaked in each loop when a
validation pattern or when a query fails, so I don't see a strong
argument in manipulating 10~14 more than necessary for this amount of
memory. The contents of describe.c are the same for v15 though, and
we are still in beta on REL_15_STABLE, so I have applied the patch
down to v15, adding what Alvaro has sent on top of the rest.

Thanks for the explanation! IMO, we could ignore v10-13 branches, however,
we should backpatch to v14 which also uses the validateSQLNamePattern()
function leading to a memory leak.

I'd agree in principle, but in practice the commit from 15 does not apply
cleanly on 14 -- there's a ton of conflicts.

I attached a patch for v14 [1]/messages/by-id/MEYP282MB166974D3883A1A6E25B5FDAEB68E9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. based on master, if you want to apply it,
please consider reviewing it.

[1]: /messages/by-id/MEYP282MB166974D3883A1A6E25B5FDAEB68E9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

#28

Michael Paquier

michael@paquier.xyz

over 3 years ago

In reply to: Japin Li (#27)

Re: Memory leak fix in psql

On Thu, Jul 21, 2022 at 02:43:03PM +0800, Japin Li wrote:

I attached a patch for v14 [1] based on master, if you want to apply it,
please consider reviewing it.

We are talking about a few hundred bytes leaked each time, so this
does not worry me much in the older branches, honestly.
--
Michael