Horton

Re: Small memory fixes for pg_createsubcriber

Euler Taveira <euler@eulerto.com>

View thread View on postgr.es
From: "Euler Taveira" <euler@eulerto.com>
To: "ranier.vf@gmail.com" <ranier.vf@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-02-11T16:32:32Z
Lists: pgsql-hackers 
On Mon, Feb 10, 2025, at 1:31 PM, Ranier Vilela wrote:
> Coverity has some reports about pg_createsubcriber.
> 
> CID 1591322: (#1 of 1): Resource leak (RESOURCE_LEAK)
> 10. leaked_storage: Variable dbname going out of scope leaks the storage it points to.
> Additionally there are several calls that are out of the ordinary, according to the Postgres API.
> 
> According to the documentation:
> libpq-exec <https://www.postgresql.org/docs/current/libpq-exec.html>
> 
> 
> The correct function to free memory when using PQescapeLiteral and PQescapeIdentifier would be PQfreemem.
> 

Hi,

>From src/common/fe_memutils.c:

void
pg_free(void *ptr)
{
    free(ptr);
}

>From src/interfaces/libpq/fe-exec.c:

void
PQfreemem(void *ptr)
{
    free(ptr);
}

There is no bug. They are the same behind the scenes. I'm fine changing it. It
is a new code and it wouldn't cause a lot of pain to backpatch patches in the
future.


@@ -1130,6 +1130,7 @@ check_and_drop_existing_subscriptions(PGconn *conn,
 
PQclear(res);
destroyPQExpBuffer(query);
+ PQfreemem(dbname);
}

Even if the pg_createsubscriber aims to run in a small amount of time, hence,
it is fine to leak memory, the initial commit cleaned up all variables but a
subsequent commit 4867f8a555c apparently didn't. Although it is just a low
impact improvement, it is better to be strict and shut up SASTs.


--
Euler Taveira
EDB   https://www.enterprisedb.com/

Commits

  1. pg_upgrade: Fix inconsistency in memory freeing

  2. pg_amcheck: Fix inconsistency in memory freeing

  3. Fix some inconsistencies with memory freeing in pg_createsubscriber

  4. Drop pre-existing subscriptions from the converted subscriber.