Silence leakage complaint about postgres_fdw's InitPgFdwOptions.

tglsfdc · tglsfdc · commit 0f9d4d7c12dc · 2025-07-25T16:37:29.000-04:00
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
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
@@ -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));
 }
