From 452c828f54312f34c6c099304007eb4b45bd6102 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Fri, 8 Mar 2024 05:57:24 +0000 Subject: [PATCH v4] Simplify backtrace_functions GUC code --- src/backend/utils/error/elog.c | 125 ++++++++++++++-------------- src/backend/utils/misc/guc_tables.c | 4 +- src/include/utils/guc_hooks.h | 1 - 3 files changed, 63 insertions(+), 67 deletions(-) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index d5fb5fd4f2..fa41a4c65e 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -114,9 +114,6 @@ char *Log_destination_string = NULL; bool syslog_sequence_numbers = true; bool syslog_split_messages = true; -/* Processed form of backtrace_functions GUC */ -static char *backtrace_function_list; - #ifdef HAVE_SYSLOG /* @@ -831,24 +828,32 @@ set_stack_entry_location(ErrorData *edata, static bool matches_backtrace_functions(const char *funcname) { - const char *p; + char *rawstring; + List *elemlist; + ListCell *l; + bool is_parase_okay PG_USED_FOR_ASSERTS_ONLY; - if (!backtrace_function_list || funcname == NULL || funcname[0] == '\0') - return false; + /* Need a modifiable copy of string */ + rawstring = pstrdup(backtrace_functions); + + /* Parse string into list of identifiers */ + is_parase_okay = SplitIdentifierString(rawstring, ',', &elemlist); + Assert(is_parase_okay); - p = backtrace_function_list; - for (;;) + foreach(l, elemlist) { - if (*p == '\0') /* end of backtrace_function_list */ - break; + char *tok = (char *) lfirst(l); - if (strcmp("*", p) == 0) - return true; - if (strcmp(funcname, p) == 0) + if (strcmp(tok, "*") == 0 || strcmp(tok, funcname) == 0) + { + pfree(rawstring); + list_free(elemlist); return true; - p += strlen(p) + 1; + } } + pfree(rawstring); + list_free(elemlist); return false; } @@ -2127,68 +2132,60 @@ DebugFileOpen(void) bool check_backtrace_functions(char **newval, void **extra, GucSource source) { - int newvallen = strlen(*newval); - char *someval; - int validlen; - int i; - int j; + char *rawstring; + List *elemlist; + ListCell *l; - /* - * Allow characters that can be C identifiers, commas as separators, the - * wildcard symbol, as well as some whitespace for readability. - */ - validlen = strspn(*newval, - "0123456789_" - "abcdefghijklmnopqrstuvwxyz" - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - ",* \n\t"); - if (validlen != newvallen) + /* Need a modifiable copy of string */ + rawstring = pstrdup(*newval); + + /* Parse string into list of identifiers */ + if (!SplitIdentifierString(rawstring, ',', &elemlist)) { - GUC_check_errdetail("Invalid character"); + /* syntax error in list */ + GUC_check_errdetail("List syntax is invalid."); + pfree(rawstring); + list_free(elemlist); return false; } - if (*newval[0] == '\0') + foreach(l, elemlist) { - *extra = NULL; - return true; - } + char *tok = (char *) lfirst(l); + int toklen = strlen(tok); + int validlen; - /* - * Allocate space for the output and create the copy. We could discount - * whitespace chars to save some memory, but it doesn't seem worth the - * trouble. - */ - someval = guc_malloc(ERROR, newvallen + 1 + 1); - for (i = 0, j = 0; i < newvallen; i++) - { - if ((*newval)[i] == ',') - someval[j++] = '\0'; /* next item */ - else if ((*newval)[i] == ' ' || - (*newval)[i] == '\n' || - (*newval)[i] == '\t') - ; /* ignore these */ - else - someval[j++] = (*newval)[i]; /* copy anything else */ - } + if (strcmp(tok, "*") == 0) + { + pfree(rawstring); + list_free(elemlist); + return true; + } + + /* + * Allow characters that can be C identifiers, commas as separators, + * the wildcard symbol, as well as some whitespace for readability. + */ + validlen = strspn(tok, + "0123456789_" + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + ", \n\t"); - /* two \0s end the setting */ - someval[j] = '\0'; - someval[j + 1] = '\0'; + if (validlen != toklen) + { + GUC_check_errdetail("Invalid character"); + pfree(rawstring); + list_free(elemlist); + return false; + } + } - *extra = someval; + pfree(rawstring); + list_free(elemlist); return true; } -/* - * GUC assign_hook for backtrace_functions - */ -void -assign_backtrace_functions(const char *newval, void *extra) -{ - backtrace_function_list = (char *) extra; -} - /* * GUC check_hook for log_destination */ diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index ff799a296a..e4ae8e1573 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -4670,11 +4670,11 @@ struct config_string ConfigureNamesString[] = {"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS, gettext_noop("Log backtrace for errors in these functions."), NULL, - GUC_NOT_IN_SAMPLE + GUC_LIST_INPUT | GUC_NOT_IN_SAMPLE }, &backtrace_functions, "", - check_backtrace_functions, assign_backtrace_functions, NULL + check_backtrace_functions, NULL, NULL }, { diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index d64dc5fcdb..b9b801216a 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -37,7 +37,6 @@ extern bool check_vacuum_buffer_usage_limit(int *newval, void **extra, GucSource source); extern bool check_backtrace_functions(char **newval, void **extra, GucSource source); -extern void assign_backtrace_functions(const char *newval, void *extra); extern bool check_bonjour(bool *newval, void **extra, GucSource source); extern bool check_canonical_path(char **newval, void **extra, GucSource source); extern void assign_checkpoint_completion_target(double newval, void *extra); -- 2.34.1