Silence leakage complaint about postgres_fdw's InitPgFdwOptions.

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
index c2f936640bca865ab4054b148775cdb032abb266..d6fa89bad93994afda5b4e1a4b1cd78ff6b106f8 100644 (file)
--- 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));
 }