From 4d8bc0e8ad06b1e9edecdd682ddf17dc6800a4dd Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 17 Jan 2025 15:41:11 +0100 Subject: [PATCH 4/4] syncrep parser: Return parse result not via global variable Instead of passing the parse result from yyparse() via a global variable, pass it via a function output argument. --- src/backend/nls.mk | 2 +- src/backend/replication/syncrep.c | 8 ++++---- src/backend/replication/syncrep_gram.y | 9 ++++----- src/backend/replication/syncrep_scanner.l | 22 +++++++++++++++++++--- src/include/replication/syncrep.h | 10 +++------- 5 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/backend/nls.mk b/src/backend/nls.mk index 210893c7b72..b7d5dd46e45 100644 --- a/src/backend/nls.mk +++ b/src/backend/nls.mk @@ -11,7 +11,7 @@ GETTEXT_TRIGGERS = $(BACKEND_COMMON_GETTEXT_TRIGGERS) \ parser_yyerror \ replication_yyerror:3 \ scanner_yyerror \ - syncrep_yyerror:2 \ + syncrep_yyerror:4 \ report_invalid_record:2 \ ereport_startup_progress \ json_token_error:2 \ diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index 1ce8bc7533f..d75e3968035 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -996,13 +996,13 @@ check_synchronous_standby_names(char **newval, void **extra, GucSource source) int parse_rc; SyncRepConfigData *pconf; - /* Reset communication variables to ensure a fresh start */ - syncrep_parse_result = NULL; - syncrep_parse_error_msg = NULL; + /* Result of parsing is returned in one of these two variables */ + SyncRepConfigData *syncrep_parse_result = NULL; + char *syncrep_parse_error_msg = NULL; /* Parse the synchronous_standby_names string */ syncrep_scanner_init(*newval, &scanner); - parse_rc = syncrep_yyparse(scanner); + parse_rc = syncrep_yyparse(&syncrep_parse_result, &syncrep_parse_error_msg, scanner); syncrep_scanner_finish(scanner); if (parse_rc != 0 || syncrep_parse_result == NULL) diff --git a/src/backend/replication/syncrep_gram.y b/src/backend/replication/syncrep_gram.y index 1a3b89ca674..22297bb151a 100644 --- a/src/backend/replication/syncrep_gram.y +++ b/src/backend/replication/syncrep_gram.y @@ -19,10 +19,6 @@ #include "syncrep_gram.h" -/* 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(const char *num_sync, List *members, uint8 syncrep_method); @@ -36,7 +32,10 @@ static SyncRepConfigData *create_syncrep_config(const char *num_sync, %} +%parse-param {SyncRepConfigData **syncrep_parse_result_p} +%parse-param {char **syncrep_parse_error_msg_p} %parse-param {yyscan_t yyscanner} +%lex-param {char **syncrep_parse_error_msg_p} %lex-param {yyscan_t yyscanner} %pure-parser %expect 0 @@ -60,7 +59,7 @@ static SyncRepConfigData *create_syncrep_config(const char *num_sync, %% result: standby_config { - syncrep_parse_result = $1; + *syncrep_parse_result_p = $1; (void) yynerrs; /* suppress compiler warning */ } ; diff --git a/src/backend/replication/syncrep_scanner.l b/src/backend/replication/syncrep_scanner.l index 05e5fdecf1b..7dec1f869c7 100644 --- a/src/backend/replication/syncrep_scanner.l +++ b/src/backend/replication/syncrep_scanner.l @@ -42,6 +42,13 @@ struct syncrep_yy_extra_type StringInfoData xdbuf; }; +/* + * Better keep this definition here than put it in replication/syncrep.h and + * save a bit of duplication. Putting it in replication/syncrep.h would leak + * the definition to other parts and possibly affect other scanners. +*/ +#define YY_DECL extern int syncrep_yylex(union YYSTYPE *yylval_param, char **syncrep_parse_error_msg_p, yyscan_t yyscanner) + /* LCOV_EXCL_START */ %} @@ -104,7 +111,7 @@ xdinside [^"]+ return NAME; } <> { - syncrep_yyerror(yyscanner, "unterminated quoted identifier"); + syncrep_yyerror(NULL, syncrep_parse_error_msg_p, yyscanner, "unterminated quoted identifier"); return JUNK; } @@ -136,12 +143,21 @@ xdinside [^"]+ #undef yyextra #define yyextra (((struct yyguts_t *) yyscanner)->yyextra_r) -/* Needs to be here for access to yytext */ +/* + * This yyerror() function does not raise an error (elog or similar), it just + * collects the error message in *syncrep_parse_error_msg_p and leaves it to + * the ultimate caller of the syncrep parser to raise the error. (The + * ultimate caller will do that with special GUC error functions.) + * + * (The first argument is enforced by Bison to match the first argument of + * yyparse(), but it is not used here.) + */ void -syncrep_yyerror(yyscan_t yyscanner, const char *message) +syncrep_yyerror(SyncRepConfigData **syncrep_parse_result_p, char **syncrep_parse_error_msg_p, yyscan_t yyscanner, const char *message) { struct yyguts_t *yyg = (struct yyguts_t *) yyscanner; /* needed for yytext * macro */ + char *syncrep_parse_error_msg = *syncrep_parse_error_msg_p; /* report only the first error in a parse operation */ if (syncrep_parse_error_msg) diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h index 33b9cdb18f7..675669a79f7 100644 --- a/src/include/replication/syncrep.h +++ b/src/include/replication/syncrep.h @@ -73,10 +73,6 @@ typedef struct SyncRepConfigData extern PGDLLIMPORT SyncRepConfigData *SyncRepConfig; -/* communication variables for parsing synchronous_standby_names GUC */ -extern PGDLLIMPORT SyncRepConfigData *syncrep_parse_result; -extern PGDLLIMPORT char *syncrep_parse_error_msg; - /* user-settable parameters for synchronous replication */ extern PGDLLIMPORT char *SyncRepStandbyNames; @@ -105,9 +101,9 @@ union YYSTYPE; #define YY_TYPEDEF_YY_SCANNER_T typedef void *yyscan_t; #endif -extern int syncrep_yyparse(yyscan_t yyscanner); -extern int syncrep_yylex(union YYSTYPE *yylval_param, yyscan_t yyscanner); -extern void syncrep_yyerror(yyscan_t yyscanner, const char *str); +extern int syncrep_yyparse(SyncRepConfigData **syncrep_parse_result_p, char **syncrep_parse_error_msg_p, yyscan_t yyscanner); +extern int syncrep_yylex(union YYSTYPE *yylval_param, char **syncrep_parse_error_msg_p, yyscan_t yyscanner); +extern void syncrep_yyerror(SyncRepConfigData **syncrep_parse_result_p, char **syncrep_parse_error_msg_p, yyscan_t yyscanner, const char *str); extern void syncrep_scanner_init(const char *str, yyscan_t *yyscannerp); extern void syncrep_scanner_finish(yyscan_t yyscanner); -- 2.47.1