Обсуждение: [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!

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

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