On Tue, 19 Jul 2022 at 20:32, Michael Paquier <mich...@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.
>From b2bcc3a1bac67b8b414f2025607f8dd35e096289 Mon Sep 17 00:00:00 2001 From: Japin Li <japi...@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