Add check on initial and boot values when loading GUCs

This commit adds a function to perform a cross-check between the initial
value of the C declaration associated to a GUC and its actual boot
value in assert-enabled builds.  The purpose of this is to prevent
anybody reading these C declarations from being fooled by mismatched
values before they are loaded at program startup.

The following rules apply depending on the GUC type:
* bool - can be false, or same as boot_val.
* int - can be 0, or same as the boot_val.
* real - can be 0.0, or same as the boot_val.
* string - can be NULL, or strcmp'd equal to the boot_val.
* enum - equal to the boot_val.

This is done for the system as well custom GUCs loaded by external
modules, which may require extension developers to adapt the C
declaration of the variables used by these GUCs (testing this change
with some of my own modules has allowed me to catch some stupid typos,
FWIW).  This may finish by being a bad experiment depending on the
feedbcak received, but let's see how it goes.

Author: Peter Smith
Reviewed-by: Nathan Bossart, Tom Lane, Michael Paquier, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 4de14efe87752c3b754009dafef640bdf1c99b19..117a2d26a0e9fa05bfce876ca170c53f41e409c7 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1382,6 +1382,89 @@ check_GUC_name_for_parameter_acl(const char *name)
        return false;
 }
 
+/*
+ * Routine in charge of checking that the initial value of a GUC is the
+ * same when declared and when loaded to prevent anybody looking at the
+ * C declarations of these GUCS from being fooled by mismatched values.
+ *
+ * The following validation rules apply:
+ * bool - can be false, otherwise must be same as the boot_val
+ * int  - can be 0, otherwise must be same as the boot_val
+ * real - can be 0.0, otherwise must be same as the boot_val
+ * string - can be NULL, otherwise must be strcmp equal to the boot_val
+ * enum - must be same as the boot_val
+ */
+#ifdef USE_ASSERT_CHECKING
+static bool
+check_GUC_init(struct config_generic *gconf)
+{
+       switch (gconf->vartype)
+       {
+               case PGC_BOOL:
+                       {
+                               struct config_bool *conf = (struct config_bool *) gconf;
+
+                               if (*conf->variable && !conf->boot_val)
+                               {
+                                       elog(LOG, "GUC (PGC_BOOL) %s, boot_val=%d, C-var=%d",
+                                                conf->gen.name, conf->boot_val, *conf->variable);
+                                       return false;
+                               }
+                               break;
+                       }
+               case PGC_INT:
+                       {
+                               struct config_int *conf = (struct config_int *) gconf;
+
+                               if (*conf->variable != 0 && *conf->variable != conf->boot_val)
+                               {
+                                       elog(LOG, "GUC (PGC_INT) %s, boot_val=%d, C-var=%d",
+                                                conf->gen.name, conf->boot_val, *conf->variable);
+                                       return false;
+                               }
+                               break;
+                       }
+               case PGC_REAL:
+                       {
+                               struct config_real *conf = (struct config_real *) gconf;
+
+                               if (*conf->variable != 0.0 && *conf->variable != conf->boot_val)
+                               {
+                                       elog(LOG, "GUC (PGC_REAL) %s, boot_val=%g, C-var=%g",
+                                                conf->gen.name, conf->boot_val, *conf->variable);
+                                       return false;
+                               }
+                               break;
+                       }
+               case PGC_STRING:
+                       {
+                               struct config_string *conf = (struct config_string *) gconf;
+
+                               if (*conf->variable != NULL && strcmp(*conf->variable, conf->boot_val) != 0)
+                               {
+                                       elog(LOG, "GUC (PGC_STRING) %s, boot_val=%s, C-var=%s",
+                                                conf->gen.name, conf->boot_val ? conf->boot_val : "<null>", *conf->variable);
+                                       return false;
+                               }
+                               break;
+                       }
+               case PGC_ENUM:
+                       {
+                               struct config_enum *conf = (struct config_enum *) gconf;
+
+                               if (*conf->variable != conf->boot_val)
+                               {
+                                       elog(LOG, "GUC (PGC_ENUM) %s, boot_val=%d, C-var=%d",
+                                                conf->gen.name, conf->boot_val, *conf->variable);
+                                       return false;
+                               }
+                               break;
+                       }
+       }
+
+       return true;
+}
+#endif
 
 /*
  * Initialize GUC options during program startup.
@@ -1413,6 +1496,9 @@ InitializeGUCOptions(void)
        hash_seq_init(&status, guc_hashtab);
        while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
        {
+               /* Check mapping between initial and default value */
+               Assert(check_GUC_init(hentry->gucvar));
+
                InitializeOneGUCOption(hentry->gucvar);
        }
 
@@ -4654,6 +4740,9 @@ define_custom_variable(struct config_generic *variable)
        GUCHashEntry *hentry;
        struct config_string *pHolder;
 
+       /* Check mapping between initial and default value */
+       Assert(check_GUC_init(variable));
+
        /*
         * See if there's a placeholder by the same name.
         */