database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-26 01:22:12 +03:00
Code Activity

Silence leakage complaint about postgres_fdw's InitPgFdwOptions.

Browse Source 
Valgrind complains that the PQconninfoOption array returned by libpq
is leaked.  We apparently believed that we could suppress that warning
by storing that array's address in a static variable.  However, modern
C compilers are bright enough to optimize the static variable away.

We could escalate that arms race by making the variable global.
But on the whole it seems better to revise the code so that it
can free libpq's result properly.  The only thing that costs
us is copying the parameter-name keywords; which seems like a
pretty negligible cost in a function that runs at most once per
process.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2025-07-25 16:37:29 -04:00
parent 73873805fb
commit 0f9d4d7c12
1 changed files with 12 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
contrib/postgres_fdw/option.c
View File

@ -21,6 +21,7 @@
#include "libpq/libpq-be.h"
#include "postgres_fdw.h"
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/varlena.h"
/*
@ -39,12 +40,6 @@ typedef struct PgFdwOption
*/
static PgFdwOption *postgres_fdw_options;
/*
* Valid options for libpq.
* Allocated and filled in InitPgFdwOptions.
*/
static PQconninfoOption *libpq_options;
/*
* GUC parameters
*/
@ -239,6 +234,7 @@ static void
InitPgFdwOptions(void)
{
int num_libpq_opts;
PQconninfoOption *libpq_options;
PQconninfoOption *lopt;
PgFdwOption *popt;
@ -307,8 +303,8 @@ InitPgFdwOptions(void)
* Get list of valid libpq options.
*
* To avoid unnecessary work, we get the list once and use it throughout
* the lifetime of this backend process. We don't need to care about
* memory context issues, because PQconndefaults allocates with malloc.
* the lifetime of this backend process. Hence, we'll allocate it in
* TopMemoryContext.
*/
libpq_options = PQconndefaults();
if (!libpq_options) /* assume reason for failure is OOM */
@ -325,19 +321,11 @@ InitPgFdwOptions(void)
/*
* Construct an array which consists of all valid options for
* postgres_fdw, by appending FDW-specific options to libpq options.
*
* We use plain malloc here to allocate postgres_fdw_options because it
* lives as long as the backend process does. Besides, keeping
* libpq_options in memory allows us to avoid copying every keyword
* string.
*/
postgres_fdw_options = (PgFdwOption *)
malloc(sizeof(PgFdwOption) * num_libpq_opts +
sizeof(non_libpq_options));
if (postgres_fdw_options == NULL)
ereport(ERROR,
(errcode(ERRCODE_FDW_OUT_OF_MEMORY),
errmsg("out of memory")));
MemoryContextAlloc(TopMemoryContext,
sizeof(PgFdwOption) * num_libpq_opts +
sizeof(non_libpq_options));
popt = postgres_fdw_options;
for (lopt = libpq_options; lopt->keyword; lopt++)
@ -355,8 +343,8 @@ InitPgFdwOptions(void)
if (strncmp(lopt->keyword, "oauth_", strlen("oauth_")) == 0)
continue;
/* We don't have to copy keyword string, as described above. */
popt->keyword = lopt->keyword;
popt->keyword = MemoryContextStrdup(TopMemoryContext,
lopt->keyword);
/*
* "user" and any secret options are allowed only on user mappings.
@ -371,6 +359,9 @@ InitPgFdwOptions(void)
popt++;
}
/* Done with libpq's output structure. */
PQconninfoFree(libpq_options);
/* Append FDW-specific options and dummy terminator. */
memcpy(popt, non_libpq_options, sizeof(non_libpq_options));
}