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:
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.
