[patch] some PQExpBuffer are not destroyed in pg_dump

Started by Zhang, Jiealmost 6 years ago4 messages
#1Zhang, Jie
zhangjie2@cn.fujitsu.com
1 attachment(s)

Hi al

In getDefaultACLs function, some PQExpBuffer are not destroy

File: src/bin/pg_dump/pg_dump.c
DefaultACLInfo *
getDefaultACLs(Archive *fout, int *numDefaultACLs)
{
......
if (fout->remoteVersion >= 90600)
{
PQExpBuffer acl_subquery = createPQExpBuffer(); // *** acl_subquery not destroyed ***
PQExpBuffer racl_subquery = createPQExpBuffer(); // *** racl_subquery not destroyed ***
PQExpBuffer initacl_subquery = createPQExpBuffer(); // *** initacl_subquery not destroyed ***
PQExpBuffer initracl_subquery = createPQExpBuffer(); // *** initracl_subquery not destroyed ***

buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
initracl_subquery, "defaclacl", "defaclrole",
"CASE WHEN defaclobjtype = 'S' THEN 's' ELSE defaclobjtype END::\"char\"",
dopt->binary_upgrade);

appendPQExpBuffer(query, "SELECT d.oid, d.tableoid, "
"(%s d.defaclrole) AS defaclrole, "
"d.defaclnamespace, "
"d.defaclobjtype, "
"%s AS defaclacl, "
"%s AS rdefaclacl, "
"%s AS initdefaclacl, "
"%s AS initrdefaclacl "
"FROM pg_default_acl d "
"LEFT JOIN pg_init_privs pip ON "
"(d.oid = pip.objoid "
"AND pip.classoid = 'pg_default_acl'::regclass "
"AND pip.objsubid = 0) ",
username_subquery,
acl_subquery->data,
racl_subquery->data,
initacl_subquery->data,
initracl_subquery->data);
}
......

Here is a patch.

Best Regards!

Attachments:

pg_dump.patchapplication/octet-stream; name=pg_dump.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 408637c..ee860c4 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -9576,6 +9576,10 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
 						  racl_subquery->data,
 						  initacl_subquery->data,
 						  initracl_subquery->data);
+		destroyPQExpBuffer(acl_subquery);
+		destroyPQExpBuffer(racl_subquery);
+		destroyPQExpBuffer(initacl_subquery);
+		destroyPQExpBuffer(initracl_subquery);
 	}
 	else
 	{
#2Masahiko Sawada
masahiko.sawada@2ndquadrant.com
In reply to: Zhang, Jie (#1)
Re: [patch] some PQExpBuffer are not destroyed in pg_dump

On Tue, 7 Apr 2020 at 11:42, Zhang, Jie <zhangjie2@cn.fujitsu.com> wrote:

Hi al

In getDefaultACLs function, some PQExpBuffer are not destroy

Yes, it looks like an oversight. It's related to the commit
e2090d9d20d809 which is back-patched to 9.6.

The patch looks good to me.

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#3Michael Paquier
michael@paquier.xyz
In reply to: Masahiko Sawada (#2)
Re: [patch] some PQExpBuffer are not destroyed in pg_dump

On Mon, Apr 13, 2020 at 04:51:06PM +0900, Masahiko Sawada wrote:

On Tue, 7 Apr 2020 at 11:42, Zhang, Jie <zhangjie2@cn.fujitsu.com> wrote:

In getDefaultACLs function, some PQExpBuffer are not destroy

Yes, it looks like an oversight. It's related to the commit
e2090d9d20d809 which is back-patched to 9.6.

The patch looks good to me.

Indeed. Any code path of pg_dump calling buildACLQueries() clears up
things, and I think that it is a better practice to clean up properly
PQExpBuffer stuff even if there is always the argument that pg_dump
is a tool running in a "short"-term context. So I will backpatch that
unless there are any objections from others.

The part I am actually rather amazed of here is that I don't recall
seeing Coverity complaining about leaks after this commit. Perhaps it
just got lost.
--
Michael

#4Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#3)
Re: [patch] some PQExpBuffer are not destroyed in pg_dump

On Tue, Apr 14, 2020 at 10:11:56AM +0900, Michael Paquier wrote:

Indeed. Any code path of pg_dump calling buildACLQueries() clears up
things, and I think that it is a better practice to clean up properly
PQExpBuffer stuff even if there is always the argument that pg_dump
is a tool running in a "short"-term context. So I will backpatch that
unless there are any objections from others.

And done as of 8f4ee44.
--
Michael