Обсуждение: [PATHC] Fix minor memory leak in pg_basebackup

Hi all

In some cases , PGresult is not cleared.

File: src\bin\pg_basebackup\streamutil.c

bool
RetrieveWalSegSize(PGconn *conn)
{
    PGresult   *res;
......
    res = PQexec(conn, "SHOW wal_segment_size");
    if (PQresultStatus(res) != PGRES_TUPLES_OK)
    {
        pg_log_error("could not send replication command \"%s\": %s",
                     "SHOW wal_segment_size", PQerrorMessage(conn));

        PQclear(res); // *** res  is cleared  ***
        return false;
    }
......
    /* fetch xlog value and unit from the result */
    if (sscanf(PQgetvalue(res, 0, 0), "%d%s", &xlog_val, xlog_unit) != 2)
    {
        pg_log_error("WAL segment size could not be parsed");
        return false;    // *** res  is not cleared  ***
    }
......
    if (!IsValidWalSegSize(WalSegSz))
    {
        pg_log_error(ngettext("WAL segment size must be a power of two between 1 MB and 1 GB, but the remote server
reporteda value of %d byte",
 
                              "WAL segment size must be a power of two between 1 MB and 1 GB, but the remote server
reporteda value of %d bytes",
 
                              WalSegSz),
                     WalSegSz);
        return false;   ;    // *** res  is not cleared  ***
    }
......


Here is a patch.

Best Regards!

On Wed, Apr 15, 2020 at 10:06:52AM +0000, Zhang, Jie wrote:
> In some cases , PGresult is not cleared.
>
> File: src\bin\pg_basebackup\streamutil.c
>
> bool
> RetrieveWalSegSize(PGconn *conn)
> {
>     PGresult   *res;

RetrieveWalSegSize() gets called only once at the beginning of
pg_basebackup and pg_receivewal, so that's not an issue that has major
effects, still that's an issue.  The first one PQclear() is needed
where you say.  Now for the second one, I would just move it once the
code is done with the query result, aka after calling PQgetvalue().
What do you think?  Please see the attached.
--
Michael

Hi  Michael

so much the better!

-----Original Message-----
From: Michael Paquier [mailto:michael@paquier.xyz] 
Sent: Thursday, April 16, 2020 2:31 PM
To: Zhang, Jie/张 杰 <zhangjie2@cn.fujitsu.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [PATHC] Fix minor memory leak in pg_basebackup

On Wed, Apr 15, 2020 at 10:06:52AM +0000, Zhang, Jie wrote:
> In some cases , PGresult is not cleared.
> 
> File: src\bin\pg_basebackup\streamutil.c
> 
> bool
> RetrieveWalSegSize(PGconn *conn)
> {
>     PGresult   *res;

RetrieveWalSegSize() gets called only once at the beginning of pg_basebackup and pg_receivewal, so that's not an issue
thathas major effects, still that's an issue.  The first one PQclear() is needed where you say.  Now for the second
one,I would just move it once the code is done with the query result, aka after calling PQgetvalue().
 
What do you think?  Please see the attached.
--
Michael

On Thu, Apr 16, 2020 at 10:54:09AM +0000, Zhang, Jie wrote:
> So much the better!

Thanks for confirming.  Fixed, then.
--
Michael