Store GUC data in a memory context, instead of using malloc().

The only real argument for using malloc directly was that we needed
the ability to not throw error on OOM; but mcxt.c grew that feature
awhile ago.

Keeping the data in a memory context improves accountability and
debuggability --- for example, without this it's almost impossible
to detect memory leaks in the GUC code with anything less costly
than valgrind.  Moreover, the next patch in this series will add a
hash table for GUC lookup, and it'd be pretty silly to be using
palloc-dependent hash facilities alongside malloc'd storage of the
underlying data.

This is a bit invasive though, in particular causing an API break
for GUC check hooks that want to modify the GUC's value or use an
"extra" data structure.  They must now use guc_malloc() and
guc_free() instead of malloc() and free().  Failure to change
affected code will result in assertion failures or worse; but
thanks to recent effort in the mcxt infrastructure, it shouldn't
be too hard to diagnose such oversights (at least in assert-enabled
builds).

One note is that this changes ParseLongOption() to return short-lived
palloc'd not malloc'd data.  There wasn't any caller for which the
previous definition was better.

Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us

Author: tglsfdc

src/backend/bootstrap/bootstrap.c

@@ -287,8 +287,8 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 					}
 
 					SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
-					free(name);
-					free(value);
+					pfree(name);
+					pfree(value);
 					break;
 				}
 			default:

src/backend/commands/tablespace.c

@@ -1290,8 +1290,8 @@ check_temp_tablespaces(char **newval, void **extra, GucSource source)
 		}
 
 		/* Now prepare an "extra" struct for assign_temp_tablespaces */
-		myextra = malloc(offsetof(temp_tablespaces_extra, tblSpcs) +
-						 numSpcs * sizeof(Oid));
+		myextra = guc_malloc(LOG, offsetof(temp_tablespaces_extra, tblSpcs) +
+							 numSpcs * sizeof(Oid));
 		if (!myextra)
 			return false;
 		myextra->numSpcs = numSpcs;

src/backend/commands/variable.c

@@ -148,15 +148,15 @@ check_datestyle(char **newval, void **extra, GucSource source)
 			char	   *subval;
 			void	   *subextra = NULL;
 
-			subval = strdup(GetConfigOptionResetString("datestyle"));
+			subval = guc_strdup(LOG, GetConfigOptionResetString("datestyle"));
 			if (!subval)
 			{
 				ok = false;
 				break;
 			}
 			if (!check_datestyle(&subval, &subextra, source))
 			{
-				free(subval);
+				guc_free(subval);
 				ok = false;
 				break;
 			}
@@ -165,8 +165,8 @@ check_datestyle(char **newval, void **extra, GucSource source)
 				newDateStyle = myextra[0];
 			if (!have_order)
 				newDateOrder = myextra[1];
-			free(subval);
-			free(subextra);
+			guc_free(subval);
+			guc_free(subextra);
 		}
 		else
 		{
@@ -187,9 +187,9 @@ check_datestyle(char **newval, void **extra, GucSource source)
 	}
 
 	/*
-	 * Prepare the canonical string to return.  GUC wants it malloc'd.
+	 * Prepare the canonical string to return.  GUC wants it guc_malloc'd.
 	 */
-	result = (char *) malloc(32);
+	result = (char *) guc_malloc(LOG, 32);
 	if (!result)
 		return false;
 
@@ -221,13 +221,13 @@ check_datestyle(char **newval, void **extra, GucSource source)
 			break;
 	}
 
-	free(*newval);
+	guc_free(*newval);
 	*newval = result;
 
 	/*
 	 * Set up the "extra" struct actually used by assign_datestyle.
 	 */
-	myextra = (int *) malloc(2 * sizeof(int));
+	myextra = (int *) guc_malloc(LOG, 2 * sizeof(int));
 	if (!myextra)
 		return false;
 	myextra[0] = newDateStyle;
@@ -366,7 +366,7 @@ check_timezone(char **newval, void **extra, GucSource source)
 	/*
 	 * Pass back data for assign_timezone to use
 	 */
-	*extra = malloc(sizeof(pg_tz *));
+	*extra = guc_malloc(LOG, sizeof(pg_tz *));
 	if (!*extra)
 		return false;
 	*((pg_tz **) *extra) = new_tz;
@@ -439,7 +439,7 @@ check_log_timezone(char **newval, void **extra, GucSource source)
 	/*
 	 * Pass back data for assign_log_timezone to use
 	 */
-	*extra = malloc(sizeof(pg_tz *));
+	*extra = guc_malloc(LOG, sizeof(pg_tz *));
 	if (!*extra)
 		return false;
 	*((pg_tz **) *extra) = new_tz;
@@ -500,7 +500,7 @@ check_timezone_abbreviations(char **newval, void **extra, GucSource source)
 		return true;
 	}
 
-	/* OK, load the file and produce a malloc'd TimeZoneAbbrevTable */
+	/* OK, load the file and produce a guc_malloc'd TimeZoneAbbrevTable */
 	*extra = load_tzoffsets(*newval);
 
 	/* tzparser.c returns NULL on failure, reporting via GUC_check_errmsg */
@@ -647,7 +647,7 @@ check_transaction_deferrable(bool *newval, void **extra, GucSource source)
 bool
 check_random_seed(double *newval, void **extra, GucSource source)
 {
-	*extra = malloc(sizeof(int));
+	*extra = guc_malloc(LOG, sizeof(int));
 	if (!*extra)
 		return false;
 	/* Arm the assign only if source of value is an interactive SET */
@@ -735,16 +735,16 @@ check_client_encoding(char **newval, void **extra, GucSource source)
 	if (strcmp(*newval, canonical_name) != 0 &&
 		strcmp(*newval, "UNICODE") != 0)
 	{
-		free(*newval);
-		*newval = strdup(canonical_name);
+		guc_free(*newval);
+		*newval = guc_strdup(LOG, canonical_name);
 		if (!*newval)
 			return false;
 	}
 
 	/*
 	 * Save the encoding's ID in *extra, for use by assign_client_encoding.
 	 */
-	*extra = malloc(sizeof(int));
+	*extra = guc_malloc(LOG, sizeof(int));
 	if (!*extra)
 		return false;
 	*((int *) *extra) = encoding;
@@ -847,7 +847,7 @@ check_session_authorization(char **newval, void **extra, GucSource source)
 	ReleaseSysCache(roleTup);
 
 	/* Set up "extra" struct for assign_session_authorization to use */
-	myextra = (role_auth_extra *) malloc(sizeof(role_auth_extra));
+	myextra = (role_auth_extra *) guc_malloc(LOG, sizeof(role_auth_extra));
 	if (!myextra)
 		return false;
 	myextra->roleid = roleid;
@@ -957,7 +957,7 @@ check_role(char **newval, void **extra, GucSource source)
 	}
 
 	/* Set up "extra" struct for assign_role to use */
-	myextra = (role_auth_extra *) malloc(sizeof(role_auth_extra));
+	myextra = (role_auth_extra *) guc_malloc(LOG, sizeof(role_auth_extra));
 	if (!myextra)
 		return false;
 	myextra->roleid = roleid;

src/backend/postmaster/postmaster.c

@@ -849,8 +849,8 @@ PostmasterMain(int argc, char *argv[])
 					}
 
 					SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
-					free(name);
-					free(value);
+					pfree(name);
+					pfree(value);
 					break;
 				}
 

src/backend/replication/syncrep.c

@@ -1054,9 +1054,9 @@ check_synchronous_standby_names(char **newval, void **extra, GucSource source)
 			return false;
 		}
 
-		/* GUC extra value must be malloc'd, not palloc'd */
+		/* GUC extra value must be guc_malloc'd, not palloc'd */
 		pconf = (SyncRepConfigData *)
-			malloc(syncrep_parse_result->config_size);
+			guc_malloc(LOG, syncrep_parse_result->config_size);
 		if (pconf == NULL)
 			return false;
 		memcpy(pconf, syncrep_parse_result, syncrep_parse_result->config_size);

src/backend/tcop/postgres.c

@@ -3871,8 +3871,8 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
 											optarg)));
 					}
 					SetConfigOption(name, value, ctx, gucsource);
-					free(name);
-					free(value);
+					pfree(name);
+					pfree(value);
 					break;
 				}
 

src/backend/utils/adt/datetime.c

@@ -29,6 +29,7 @@
 #include "utils/builtins.h"
 #include "utils/date.h"
 #include "utils/datetime.h"
+#include "utils/guc.h"
 #include "utils/memutils.h"
 #include "utils/tzparser.h"
 
@@ -4782,8 +4783,8 @@ TemporalSimplify(int32 max_precis, Node *node)
  * to create the final array of timezone tokens.  The argument array
  * is already sorted in name order.
  *
- * The result is a TimeZoneAbbrevTable (which must be a single malloc'd chunk)
- * or NULL on malloc failure.  No other error conditions are defined.
+ * The result is a TimeZoneAbbrevTable (which must be a single guc_malloc'd
+ * chunk) or NULL on alloc failure.  No other error conditions are defined.
  */
 TimeZoneAbbrevTable *
 ConvertTimeZoneAbbrevs(struct tzEntry *abbrevs, int n)
@@ -4812,7 +4813,7 @@ ConvertTimeZoneAbbrevs(struct tzEntry *abbrevs, int n)
 	}
 
 	/* Alloc the result ... */
-	tbl = malloc(tbl_size);
+	tbl = guc_malloc(LOG, tbl_size);
 	if (!tbl)
 		return NULL;
 

src/backend/utils/cache/ts_cache.c

@@ -633,9 +633,9 @@ check_default_text_search_config(char **newval, void **extra, GucSource source)
 
 		ReleaseSysCache(tuple);
 
-		/* GUC wants it malloc'd not palloc'd */
-		free(*newval);
-		*newval = strdup(buf);
+		/* GUC wants it guc_malloc'd not palloc'd */
+		guc_free(*newval);
+		*newval = guc_strdup(LOG, buf);
 		pfree(buf);
 		if (!*newval)
 			return false;

src/backend/utils/misc/README

@@ -51,13 +51,13 @@ out-of-memory.
 This might be used for example to canonicalize the spelling of a string
 value, round off a buffer size to the nearest supported value, or replace
 a special value such as "-1" with a computed default value.  If the
-function wishes to replace a string value, it must malloc (not palloc)
-the replacement value, and be sure to free() the previous value.
+function wishes to replace a string value, it must guc_malloc (not palloc)
+the replacement value, and be sure to guc_free() the previous value.
 
 * Derived information, such as the role OID represented by a user name,
-can be stored for use by the assign hook.  To do this, malloc (not palloc)
+can be stored for use by the assign hook.  To do this, guc_malloc (not palloc)
 storage space for the information, and return its address at *extra.
-guc.c will automatically free() this space when the associated GUC setting
+guc.c will automatically guc_free() this space when the associated GUC setting
 is no longer of interest.  *extra is initialized to NULL before call, so
 it can be ignored if not needed.
 
@@ -255,10 +255,9 @@ maintained by GUC.
 GUC Memory Handling
 -------------------
 
-String variable values are allocated with malloc/strdup, not with the
-palloc/pstrdup mechanisms.  We would need to keep them in a permanent
-context anyway, and malloc gives us more control over handling
-out-of-memory failures.
+String variable values are allocated with guc_malloc or guc_strdup,
+which ensure that the values are kept in a long-lived context, and provide
+more control over handling out-of-memory failures than bare palloc.
 
 We allow a string variable's actual value, reset_val, boot_val, and stacked
 values to point at the same storage.  This makes it slightly harder to free