Thread: [patch] some PQExpBuffer are not destroyed in pg_dump
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!
Attachment
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
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
Attachment
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