From 4c804fbdfb472cf71db33609258b8e1aaad81943 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 27 Apr 2016 17:55:19 -0400
Subject: [PATCH] Clean up parsing of synchronous_standby_names GUC variable.

Commit 989be0810dffd08b added a flex/bison lexer/parser to interpret
synchronous_standby_names.  It was done in a pretty crufty way, though,
making assorted end-use sites responsible for calling the parser at the
right times.  That was not only vulnerable to errors of omission, but made
it possible that lexer/parser errors occur at very undesirable times,
and created memory leakages even if there was no error.

Instead, perform the parsing once during check_synchronous_standby_names
and let guc.c manage the resulting data.  To do that, we have to flatten
the parsed representation into a single hunk of malloc'd memory, but that
is not very hard.

While at it, work a little harder on making useful error reports for
parsing problems; the previous code felt that "synchronous_standby_names
parser returned 1" was an appropriate user-facing error message.  (To
be fair, it did also log a syntax error message, but separately from the
GUC problem report, which is at best confusing.)  It had some outright
bugs in the face of invalid input, too.

I (tgl) also concluded that we need to restrict unquoted names in
synchronous_standby_names to be just SQL identifiers.  The previous coding
would accept darn near anything, which (1) makes the quoting convention
both nearly-unnecessary and formally ambiguous, (2) makes it very hard to
understand what is a syntax error and what is a creative interpretation of
the input as a standby name, and (3) makes it impossible to further extend
the syntax in future without a compatibility break.  I presume that we're
intending future extensions of the syntax, else this parsing infrastructure
is massive overkill, so (3) is an important objection.  Since we've taken
a compatibility hit for non-identifier names with this change anyway, we
might as well lock things down now and insist that users use double quotes
for standby names that aren't identifiers.

Kyotaro Horiguchi and Tom Lane
---
 doc/src/sgml/config.sgml                  |  27 ++--
 src/backend/replication/syncrep.c         | 168 ++++++++++------------
 src/backend/replication/syncrep_gram.y    |  68 ++++++---
 src/backend/replication/syncrep_scanner.l |  99 +++++++------
 src/backend/replication/walsender.c       |  13 +-
 src/backend/utils/misc/guc.c              |   2 +-
 src/include/replication/syncrep.h         |  25 ++--
 7 files changed, 208 insertions(+), 194 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index dce1daa73ae..b2c5e4d5dd2 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2983,7 +2983,7 @@ include_dir 'conf.d'
       </term>
       <listitem>
        <para>
-        Specifies a list of standby names that can support
+        Specifies a list of standby servers that can support
         <firstterm>synchronous replication</>, as described in
         <xref linkend="synchronous-replication">.
         There will be one or more active synchronous standbys;
@@ -2991,7 +2991,7 @@ include_dir 'conf.d'
         these standby servers confirm receipt of their data.
         The synchronous standbys will be those whose names appear
         earlier in this list, and
-        that is both currently connected and streaming data in real-time
+        that are both currently connected and streaming data in real-time
         (as shown by a state of <literal>streaming</literal> in the
         <link linkend="monitoring-stats-views-table">
         <literal>pg_stat_replication</></link> view).
@@ -3002,7 +3002,7 @@ include_dir 'conf.d'
         Specifying more than one standby name can allow very high availability.
        </para>
        <para>
-        This parameter specifies a list of standby servers by using
+        This parameter specifies a list of standby servers using
         either of the following syntaxes:
 <synopsis>
 <replaceable class="parameter">num_sync</replaceable> ( <replaceable class="parameter">standby_name</replaceable> [, ...] )
@@ -3013,17 +3013,17 @@ include_dir 'conf.d'
         wait for replies from,
         and <replaceable class="parameter">standby_name</replaceable>
         is the name of a standby server. For example, a setting of
-        <literal>'3 (s1, s2, s3, s4)'</> makes transaction commits wait
-        until their WAL records are received by three higher priority standbys
+        <literal>3 (s1, s2, s3, s4)</> makes transaction commits wait
+        until their WAL records are received by three higher-priority standbys
         chosen from standby servers <literal>s1</>, <literal>s2</>,
         <literal>s3</> and <literal>s4</>.
         </para>
         <para>
         The second syntax was used before <productname>PostgreSQL</>
         version 9.6 and is still supported. It's the same as the first syntax
-        with <replaceable class="parameter">num_sync</replaceable>=1.
-        For example, both settings of <literal>'1 (s1, s2)'</> and
-        <literal>'s1, s2'</> have the same meaning; either <literal>s1</>
+        with <replaceable class="parameter">num_sync</replaceable> equal to 1.
+        For example, <literal>1 (s1, s2)</> and
+        <literal>s1, s2</> have the same meaning: either <literal>s1</>
         or <literal>s2</> is chosen as a synchronous standby.
        </para>
        <para>
@@ -3039,11 +3039,12 @@ include_dir 'conf.d'
        </para>
        <note>
         <para>
-         The <replaceable class="parameter">standby_name</replaceable>
-         must be enclosed in double quotes if a comma (<literal>,</>),
-         a double quote (<literal>"</>), <!-- " font-lock sanity -->
-         a left parentheses (<literal>(</>), a right parentheses (<literal>)</>)
-         or a space is used in the name of a standby server.
+         Each <replaceable class="parameter">standby_name</replaceable>
+         should have the form of a valid SQL identifier, unless it
+         is <literal>*</>.  You can use double-quoting if necessary.  But note
+         that <replaceable class="parameter">standby_name</replaceable>s are
+         compared to standby application names case-insensitively, whether
+         double-quoted or not.
         </para>
        </note>
        <para>
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 3c9142eddca..acdbf1e230c 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -78,7 +78,7 @@ char	   *SyncRepStandbyNames;
 
 static bool announce_next_takeover = true;
 
-SyncRepConfigData *SyncRepConfig;
+static SyncRepConfigData *SyncRepConfig = NULL;
 static int	SyncRepWaitMode = SYNC_REP_NO_WAIT;
 
 static void SyncRepQueueInsert(int mode);
@@ -361,11 +361,6 @@ SyncRepInitConfig(void)
 {
 	int			priority;
 
-	/* Update the config data of synchronous replication */
-	SyncRepFreeConfig(SyncRepConfig);
-	SyncRepConfig = NULL;
-	SyncRepUpdateConfig();
-
 	/*
 	 * Determine if we are a potential sync standby and remember the result
 	 * for handling replies from standby.
@@ -509,7 +504,9 @@ SyncRepGetOldestSyncRecPtr(XLogRecPtr *writePtr, XLogRecPtr *flushPtr,
 	 * Quick exit if we are not managing a sync standby or there are not
 	 * enough synchronous standbys.
 	 */
-	if (!(*am_sync) || list_length(sync_standbys) < SyncRepConfig->num_sync)
+	if (!(*am_sync) ||
+		SyncRepConfig == NULL ||
+		list_length(sync_standbys) < SyncRepConfig->num_sync)
 	{
 		list_free(sync_standbys);
 		return false;
@@ -568,14 +565,15 @@ SyncRepGetSyncStandbys(bool *am_sync)
 	volatile WalSnd	*walsnd;		/* Use volatile pointer to prevent
 									 * code rearrangement */
 
+	/* Set default result */
+	if (am_sync != NULL)
+		*am_sync = false;
+
 	/* Quick exit if sync replication is not requested */
 	if (SyncRepConfig == NULL)
 		return NIL;
 
-	if (am_sync != NULL)
-		*am_sync = false;
-
-	lowest_priority = list_length(SyncRepConfig->members);
+	lowest_priority = SyncRepConfig->nmembers;
 	next_highest_priority = lowest_priority + 1;
 
 	/*
@@ -730,9 +728,8 @@ SyncRepGetSyncStandbys(bool *am_sync)
 static int
 SyncRepGetStandbyPriority(void)
 {
-	List	   *members;
-	ListCell   *l;
-	int			priority = 0;
+	const char *standby_name;
+	int			priority;
 	bool		found = false;
 
 	/*
@@ -742,22 +739,19 @@ SyncRepGetStandbyPriority(void)
 	if (am_cascading_walsender)
 		return 0;
 
-	if (!SyncStandbysDefined())
+	if (!SyncStandbysDefined() || SyncRepConfig == NULL)
 		return 0;
 
-	members = SyncRepConfig->members;
-	foreach(l, members)
+	standby_name = SyncRepConfig->member_names;
+	for (priority = 1; priority <= SyncRepConfig->nmembers; priority++)
 	{
-		char	   *standby_name = (char *) lfirst(l);
-
-		priority++;
-
 		if (pg_strcasecmp(standby_name, application_name) == 0 ||
-			pg_strcasecmp(standby_name, "*") == 0)
+			strcmp(standby_name, "*") == 0)
 		{
 			found = true;
 			break;
 		}
+		standby_name += strlen(standby_name) + 1;
 	}
 
 	return (found ? priority : 0);
@@ -867,50 +861,6 @@ SyncRepUpdateSyncStandbysDefined(void)
 	}
 }
 
-/*
- * Parse synchronous_standby_names and update the config data
- * of synchronous standbys.
- */
-void
-SyncRepUpdateConfig(void)
-{
-	int	parse_rc;
-
-	if (!SyncStandbysDefined())
-		return;
-
-	/*
-	 * check_synchronous_standby_names() verifies the setting value of
-	 * synchronous_standby_names before this function is called. So
-	 * syncrep_yyparse() must not cause an error here.
-	 */
-	syncrep_scanner_init(SyncRepStandbyNames);
-	parse_rc = syncrep_yyparse();
-	syncrep_scanner_finish();
-
-	if (parse_rc != 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg_internal("synchronous_standby_names parser returned %d",
-								 parse_rc)));
-
-	SyncRepConfig = syncrep_parse_result;
-	syncrep_parse_result = NULL;
-}
-
-/*
- * Free a previously-allocated config data of synchronous replication.
- */
-void
-SyncRepFreeConfig(SyncRepConfigData *config)
-{
-	if (!config)
-		return;
-
-	list_free_deep(config->members);
-	pfree(config);
-}
-
 #ifdef USE_ASSERT_CHECKING
 static bool
 SyncRepQueueIsOrderedByLSN(int mode)
@@ -955,78 +905,104 @@ SyncRepQueueIsOrderedByLSN(int mode)
 bool
 check_synchronous_standby_names(char **newval, void **extra, GucSource source)
 {
-	int	parse_rc;
-
 	if (*newval != NULL && (*newval)[0] != '\0')
 	{
+		int			parse_rc;
+		SyncRepConfigData *pconf;
+
+		/* Reset communication variables to ensure a fresh start */
+		syncrep_parse_result = NULL;
+		syncrep_parse_error_msg = NULL;
+
+		/* Parse the synchronous_standby_names string */
 		syncrep_scanner_init(*newval);
 		parse_rc = syncrep_yyparse();
 		syncrep_scanner_finish();
 
-		if (parse_rc != 0)
+		if (parse_rc != 0 || syncrep_parse_result == NULL)
 		{
 			GUC_check_errcode(ERRCODE_SYNTAX_ERROR);
-			GUC_check_errdetail("synchronous_standby_names parser returned %d",
-								parse_rc);
+			if (syncrep_parse_error_msg)
+				GUC_check_errdetail("%s", syncrep_parse_error_msg);
+			else
+				GUC_check_errdetail("synchronous_standby_names parser failed");
 			return false;
 		}
 
 		/*
 		 * Warn if num_sync exceeds the number of names of potential sync
-		 * standbys. This setting doesn't make sense in most cases because
-		 * it implies that enough number of sync standbys will not appear,
-		 * which makes transaction commits wait for sync replication
-		 * infinitely.
+		 * standbys. This setting doesn't make sense in most cases because it
+		 * implies that enough number of sync standbys will not appear, which
+		 * makes transaction commits wait for sync replication infinitely.
 		 *
 		 * If there are more than one standbys having the same name and
 		 * priority, we can see enough sync standbys to complete transaction
-		 * commits. However it's not recommended to run multiple standbys
-		 * with the same priority because we cannot gain full control of
-		 * the selection of sync standbys from them.
+		 * commits. However it's not recommended to run multiple standbys with
+		 * the same priority because we cannot gain full control of the
+		 * selection of sync standbys from them.
 		 *
 		 * OTOH, that setting is OK if we understand the above problem
-		 * regarding the selection of sync standbys and intentionally
-		 * specify * to match all the standbys.
+		 * regarding the selection of sync standbys and intentionally specify *
+		 * to match all the standbys.
 		 */
-		if (syncrep_parse_result->num_sync >
-			list_length(syncrep_parse_result->members))
+		if (syncrep_parse_result->num_sync > syncrep_parse_result->nmembers)
 		{
-			ListCell	*l;
-			bool			has_asterisk = false;
+			const char *standby_name;
+			int			i;
+			bool		has_asterisk = false;
 
-			foreach(l, syncrep_parse_result->members)
+			standby_name = syncrep_parse_result->member_names;
+			for (i = 1; i <= syncrep_parse_result->nmembers; i++)
 			{
-				char		*standby_name = (char *) lfirst(l);
-
-				if (pg_strcasecmp(standby_name, "*") == 0)
+				if (strcmp(standby_name, "*") == 0)
 				{
 					has_asterisk = true;
 					break;
 				}
+				standby_name += strlen(standby_name) + 1;
 			}
 
 			/*
-			 * Only the postmaster warns this inappropriate setting
-			 * to avoid cluttering the log.
+			 * Only the postmaster warns about this inappropriate setting to
+			 * avoid cluttering the log.
 			 */
 			if (!has_asterisk && !IsUnderPostmaster)
 				ereport(WARNING,
-						(errmsg("The configured number of synchronous standbys (%d) exceeds the number of names of potential synchronous ones (%d)",
-								syncrep_parse_result->num_sync, list_length(syncrep_parse_result->members)),
+						(errmsg("configured number of synchronous standbys (%d) exceeds the number of names of potential synchronous ones (%d)",
+								syncrep_parse_result->num_sync,
+								syncrep_parse_result->nmembers),
 						 errhint("Specify more names of potential synchronous standbys in synchronous_standby_names.")));
 		}
 
+		/* GUC extra value must be malloc'd, not palloc'd */
+		pconf = (SyncRepConfigData *)
+			malloc(syncrep_parse_result->config_size);
+		if (pconf == NULL)
+			return false;
+		memcpy(pconf, syncrep_parse_result, syncrep_parse_result->config_size);
+
+		*extra = (void *) pconf;
+
 		/*
-		 * syncrep_yyparse sets the global syncrep_parse_result as side effect.
-		 * But this function is required to just check, so frees it
-		 * after parsing the parameter.
+		 * We need not explicitly clean up syncrep_parse_result.  It, and any
+		 * other cruft generated during parsing, will be freed when the
+		 * current memory context is deleted.  (This code is generally run in
+		 * a short-lived context used for config file processing, so that will
+		 * not be very long.)
 		 */
-		SyncRepFreeConfig(syncrep_parse_result);
 	}
+	else
+		*extra = NULL;
 
 	return true;
 }
 
+void
+assign_synchronous_standby_names(const char *newval, void *extra)
+{
+	SyncRepConfig = (SyncRepConfigData *) extra;
+}
+
 void
 assign_synchronous_commit(int newval, void *extra)
 {
diff --git a/src/backend/replication/syncrep_gram.y b/src/backend/replication/syncrep_gram.y
index 380fedc1712..35c27760d12 100644
--- a/src/backend/replication/syncrep_gram.y
+++ b/src/backend/replication/syncrep_gram.y
@@ -12,16 +12,16 @@
  *
  *-------------------------------------------------------------------------
  */
-
 #include "postgres.h"
 
 #include "replication/syncrep.h"
-#include "utils/formatting.h"
 
-/* Result of the parsing is returned here */
-SyncRepConfigData	*syncrep_parse_result;
+/* Result of parsing is returned in one of these two variables */
+SyncRepConfigData *syncrep_parse_result;
+char	   *syncrep_parse_error_msg;
 
-static SyncRepConfigData *create_syncrep_config(char *num_sync, List *members);
+static SyncRepConfigData *create_syncrep_config(const char *num_sync,
+					  List *members);
 
 /*
  * Bison doesn't allocate anything that needs to live across parser calls,
@@ -43,10 +43,10 @@ static SyncRepConfigData *create_syncrep_config(char *num_sync, List *members);
 {
 	char	   *str;
 	List	   *list;
-	SyncRepConfigData  *config;
+	SyncRepConfigData *config;
 }
 
-%token <str> NAME NUM
+%token <str> NAME NUM JUNK
 
 %type <config> result standby_config
 %type <list> standby_list
@@ -57,29 +57,57 @@ static SyncRepConfigData *create_syncrep_config(char *num_sync, List *members);
 %%
 result:
 		standby_config				{ syncrep_parse_result = $1; }
-;
+	;
+
 standby_config:
 		standby_list				{ $$ = create_syncrep_config("1", $1); }
-		| NUM '(' standby_list ')'		{ $$ = create_syncrep_config($1, $3); }
-;
+		| NUM '(' standby_list ')'	{ $$ = create_syncrep_config($1, $3); }
+	;
+
 standby_list:
-		standby_name				{ $$ = list_make1($1);}
-		| standby_list ',' standby_name		{ $$ = lappend($1, $3);}
-;
+		standby_name						{ $$ = list_make1($1); }
+		| standby_list ',' standby_name		{ $$ = lappend($1, $3); }
+	;
+
 standby_name:
-		NAME					{ $$ = $1; }
-		| NUM					{ $$ = $1; }
-;
+		NAME						{ $$ = $1; }
+		| NUM						{ $$ = $1; }
+	;
 %%
 
+
 static SyncRepConfigData *
-create_syncrep_config(char *num_sync, List *members)
+create_syncrep_config(const char *num_sync, List *members)
 {
-	SyncRepConfigData *config =
-		(SyncRepConfigData *) palloc(sizeof(SyncRepConfigData));
+	SyncRepConfigData *config;
+	int			size;
+	ListCell   *lc;
+	char	   *ptr;
 
+	/* Compute space needed for flat representation */
+	size = offsetof(SyncRepConfigData, member_names);
+	foreach(lc, members)
+	{
+		char	   *standby_name = (char *) lfirst(lc);
+
+		size += strlen(standby_name) + 1;
+	}
+
+	/* And transform the data into flat representation */
+	config = (SyncRepConfigData *) palloc(size);
+
+	config->config_size = size;
 	config->num_sync = atoi(num_sync);
-	config->members = members;
+	config->nmembers = list_length(members);
+	ptr = config->member_names;
+	foreach(lc, members)
+	{
+		char	   *standby_name = (char *) lfirst(lc);
+
+		strcpy(ptr, standby_name);
+		ptr += strlen(standby_name) + 1;
+	}
+
 	return config;
 }
 
diff --git a/src/backend/replication/syncrep_scanner.l b/src/backend/replication/syncrep_scanner.l
index 968265e3bbb..d20662ed038 100644
--- a/src/backend/replication/syncrep_scanner.l
+++ b/src/backend/replication/syncrep_scanner.l
@@ -15,31 +15,16 @@
  */
 #include "postgres.h"
 
-#include "miscadmin.h"
 #include "lib/stringinfo.h"
 
-/*
-  * flex emits a yy_fatal_error() function that it calls in response to
-  * critical errors like malloc failure, file I/O errors, and detection of
-  * internal inconsistency.  That function prints a message and calls exit().
-  * Mutate it to instead call ereport(FATAL), which terminates this process.
-  *
-  * The process that causes this fatal error should be terminated.
-  * Otherwise it has to abandon the new setting value of
-  * synchronous_standby_names and keep running with the previous one
-  * while the other processes switch to the new one.
-  * This inconsistency of the setting that each process is based on
-  * can cause a serious problem. Though it's basically not good idea to
-  * use FATAL here because it can take down the postmaster,
-  * we should do that in order to avoid such an inconsistency.
-  */
+/* Avoid exit() on fatal scanner errors (a bit ugly -- see yy_fatal_error) */
 #undef fprintf
-#define fprintf(file, fmt, msg) syncrep_flex_fatal(fmt, msg)
+#define fprintf(file, fmt, msg)  fprintf_to_ereport(fmt, msg)
 
 static void
-syncrep_flex_fatal(const char *fmt, const char *msg)
+fprintf_to_ereport(const char *fmt, const char *msg)
 {
-	ereport(FATAL, (errmsg_internal("%s", msg)));
+	ereport(ERROR, (errmsg_internal("%s", msg)));
 }
 
 /* Handles to the buffer that the lexer uses internally */
@@ -51,8 +36,9 @@ static StringInfoData xdbuf;
 
 %option 8bit
 %option never-interactive
-%option nounput
+%option nodefault
 %option noinput
+%option nounput
 %option noyywrap
 %option warn
 %option prefix="syncrep_yy"
@@ -62,57 +48,79 @@ static StringInfoData xdbuf;
  */
 %x xd
 
-space		[ \t\n\r\f\v]
+space			[ \t\n\r\f\v]
 
-undquoted_start	[^ ,\(\)\"]
-undquoted_cont		[^ ,\(\)]
-undquoted_name    {undquoted_start}{undquoted_cont}*
-dquoted_name		[^\"]+
+digit			[0-9]
+ident_start		[A-Za-z\200-\377_]
+ident_cont		[A-Za-z\200-\377_0-9\$]
+identifier		{ident_start}{ident_cont}*
 
-/* Double-quoted string */
-dquote		\"
-xdstart		{dquote}
+dquote			\"
+xdstart			{dquote}
+xdstop			{dquote}
 xddouble		{dquote}{dquote}
-xdstop		{dquote}
-xdinside		{dquoted_name}
+xdinside		[^"]+
 
 %%
-{space}+		{ /* ignore */ }
+{space}+	{ /* ignore */ }
+
 {xdstart}	{
 				initStringInfo(&xdbuf);
 				BEGIN(xd);
 		}
 <xd>{xddouble} {
-				appendStringInfoChar(&xdbuf, '\"');
+				appendStringInfoChar(&xdbuf, '"');
 		}
 <xd>{xdinside} {
 				appendStringInfoString(&xdbuf, yytext);
 		}
 <xd>{xdstop} {
-				yylval.str = pstrdup(xdbuf.data);
-				pfree(xdbuf.data);
+				yylval.str = xdbuf.data;
+				xdbuf.data = NULL;
 				BEGIN(INITIAL);
 				return NAME;
 		}
-","			{ return ','; }
-"("			{ return '('; }
-")"			{ return ')'; }
-[1-9][0-9]*	{
+<xd><<EOF>> {
+				yyerror("unterminated quoted identifier");
+				return JUNK;
+		}
+
+{identifier} {
 				yylval.str = pstrdup(yytext);
-				return NUM;
+				return NAME;
 		}
-{undquoted_name} {
+
+{digit}+	{
 				yylval.str = pstrdup(yytext);
+				return NUM;
+		}
+
+"*"		{
+				yylval.str = "*";
 				return NAME;
 		}
+
+","			{ return ','; }
+"("			{ return '('; }
+")"			{ return ')'; }
+
+.			{ return JUNK; }
 %%
 
+
+/* Needs to be here for access to yytext */
 void
-yyerror(const char *message)
+syncrep_yyerror(const char *message)
 {
-	ereport(IsUnderPostmaster ? DEBUG2 : LOG,
-			(errcode(ERRCODE_SYNTAX_ERROR),
-			 errmsg("%s at or near \"%s\"", message, yytext)));
+	/* report only the first error in a parse operation */
+	if (syncrep_parse_error_msg)
+		return;
+	if (yytext[0])
+		syncrep_parse_error_msg = psprintf("%s at or near \"%s\"",
+										   message, yytext);
+	else
+		syncrep_parse_error_msg = psprintf("%s at end of input",
+										   message);
 }
 
 void
@@ -134,6 +142,9 @@ syncrep_scanner_init(const char *str)
 	memcpy(scanbuf, str, slen);
 	scanbuf[slen] = scanbuf[slen + 1] = YY_END_OF_BUFFER_CHAR;
 	scanbufhandle = yy_scan_buffer(scanbuf, slen + 2);
+
+	/* Make sure we start in proper state */
+	BEGIN(INITIAL);
 }
 
 void
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 81d3d285c90..20d23d5a26a 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2780,23 +2780,12 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
 	MemoryContextSwitchTo(oldcontext);
 
 	/*
-	 * Allocate and update the config data of synchronous replication,
-	 * and then get the currently active synchronous standbys.
+	 * Get the currently active synchronous standbys.
 	 */
-	SyncRepUpdateConfig();
 	LWLockAcquire(SyncRepLock, LW_SHARED);
 	sync_standbys = SyncRepGetSyncStandbys(NULL);
 	LWLockRelease(SyncRepLock);
 
-	/*
-	 * Free the previously-allocated config data because a backend
-	 * no longer needs it. The next call of this function needs to
-	 * allocate and update the config data newly because the setting
-	 * of sync replication might be changed between the calls.
-	 */
-	SyncRepFreeConfig(SyncRepConfig);
-	SyncRepConfig = NULL;
-
 	for (i = 0; i < max_wal_senders; i++)
 	{
 		WalSnd *walsnd = &WalSndCtl->walsnds[i];
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 26ca06cfc25..752f823a164 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3484,7 +3484,7 @@ static struct config_string ConfigureNamesString[] =
 		},
 		&SyncRepStandbyNames,
 		"",
-		check_synchronous_standby_names, NULL, NULL
+		check_synchronous_standby_names, assign_synchronous_standby_names, NULL
 	},
 
 	{
diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h
index 14b56649da1..e4e0e273714 100644
--- a/src/include/replication/syncrep.h
+++ b/src/include/replication/syncrep.h
@@ -20,7 +20,7 @@
 	(max_wal_senders > 0 && synchronous_commit > SYNCHRONOUS_COMMIT_LOCAL_FLUSH)
 
 /* SyncRepWaitMode */
-#define SYNC_REP_NO_WAIT		-1
+#define SYNC_REP_NO_WAIT		(-1)
 #define SYNC_REP_WAIT_WRITE		0
 #define SYNC_REP_WAIT_FLUSH		1
 #define SYNC_REP_WAIT_APPLY		2
@@ -34,15 +34,24 @@
 
 /*
  * Struct for the configuration of synchronous replication.
+ *
+ * Note: this must be a flat representation that can be held in a single
+ * chunk of malloc'd memory, so that it can be stored as the "extra" data
+ * for the synchronous_standby_names GUC.
  */
 typedef struct SyncRepConfigData
 {
-	int	num_sync;	/* number of sync standbys that we need to wait for */
-	List	*members;	/* list of names of potential sync standbys */
+	int			config_size;	/* total size of this struct, in bytes */
+	int			num_sync;		/* number of sync standbys that we need to
+								 * wait for */
+	int			nmembers;		/* number of members in the following list */
+	/* member_names contains nmembers consecutive nul-terminated C strings */
+	char		member_names[FLEXIBLE_ARRAY_MEMBER];
 } SyncRepConfigData;
 
+/* communication variables for parsing synchronous_standby_names GUC */
 extern SyncRepConfigData *syncrep_parse_result;
-extern SyncRepConfigData *SyncRepConfig;
+extern char *syncrep_parse_error_msg;
 
 /* user-settable parameters for synchronous replication */
 extern char *SyncRepStandbyNames;
@@ -59,21 +68,21 @@ extern void SyncRepReleaseWaiters(void);
 
 /* called by wal sender and user backend */
 extern List *SyncRepGetSyncStandbys(bool *am_sync);
-extern void SyncRepUpdateConfig(void);
-extern void SyncRepFreeConfig(SyncRepConfigData *config);
 
 /* called by checkpointer */
 extern void SyncRepUpdateSyncStandbysDefined(void);
 
+/* GUC infrastructure */
 extern bool check_synchronous_standby_names(char **newval, void **extra, GucSource source);
+extern void assign_synchronous_standby_names(const char *newval, void *extra);
 extern void assign_synchronous_commit(int newval, void *extra);
 
 /*
  * Internal functions for parsing synchronous_standby_names grammar,
  * in syncrep_gram.y and syncrep_scanner.l
  */
-extern int  syncrep_yyparse(void);
-extern int  syncrep_yylex(void);
+extern int	syncrep_yyparse(void);
+extern int	syncrep_yylex(void);
 extern void syncrep_yyerror(const char *str);
 extern void syncrep_scanner_init(const char *query_string);
 extern void syncrep_scanner_finish(void);
-- 
GitLab