diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index a9a2fdb..61b2304 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -254,7 +254,7 @@ exec_command(const char *cmd, if (opt1 != NULL && strncmp(opt1, prefix, sizeof(prefix) - 1) == 0) { reuse_previous = - ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix) ? + ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix, NULL) ? TRI_YES : TRI_NO; free(opt1); @@ -1548,7 +1548,7 @@ exec_command(const char *cmd, OT_NORMAL, NULL, false); if (opt) - pset.timing = ParseVariableBool(opt, "\\timing"); + pset.timing = ParseVariableBool(opt, "\\timing", NULL); else pset.timing = !pset.timing; if (!pset.quiet) @@ -2660,7 +2660,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) if (value && pg_strcasecmp(value, "auto") == 0) popt->topt.expanded = 2; else if (value) - popt->topt.expanded = ParseVariableBool(value, param); + popt->topt.expanded = ParseVariableBool(value, param, NULL); else popt->topt.expanded = !popt->topt.expanded; } @@ -2669,7 +2669,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) else if (strcmp(param, "numericlocale") == 0) { if (value) - popt->topt.numericLocale = ParseVariableBool(value, param); + popt->topt.numericLocale = ParseVariableBool(value, param, NULL); else popt->topt.numericLocale = !popt->topt.numericLocale; } @@ -2724,7 +2724,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) else if (strcmp(param, "t") == 0 || strcmp(param, "tuples_only") == 0) { if (value) - popt->topt.tuples_only = ParseVariableBool(value, param); + popt->topt.tuples_only = ParseVariableBool(value, param, NULL); else popt->topt.tuples_only = !popt->topt.tuples_only; } @@ -2756,7 +2756,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) popt->topt.pager = 2; else if (value) { - if (ParseVariableBool(value, param)) + if (ParseVariableBool(value, param, NULL)) popt->topt.pager = 1; else popt->topt.pager = 0; @@ -2778,7 +2778,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) else if (strcmp(param, "footer") == 0) { if (value) - popt->topt.default_footer = ParseVariableBool(value, param); + popt->topt.default_footer = ParseVariableBool(value, param, NULL); else popt->topt.default_footer = !popt->topt.default_footer; } diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 7ce05fb..9dcfc0a 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -786,43 +786,59 @@ showVersion(void) * This isn't an amazingly good place for them, but neither is anywhere else. */ -static void +/* + * Hook to set an internal flag from a user-supplied string value. + * If the syntax is correct, affect *flag and return true. + * Otherwise, keep *flag untouched and return false. + */ +static bool +generic_boolean_hook(const char *newval, const char* varname, bool *flag) +{ + bool isvalid; + bool val = ParseVariableBool(newval, varname, &isvalid); + if (isvalid) + *flag = val; + return isvalid; +} + +static bool autocommit_hook(const char *newval) { - pset.autocommit = ParseVariableBool(newval, "AUTOCOMMIT"); + return generic_boolean_hook(newval, "AUTOCOMMIT", &pset.autocommit); } -static void +static bool on_error_stop_hook(const char *newval) { - pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP"); + return generic_boolean_hook(newval, "ON_ERROR_STOP", &pset.on_error_stop); } -static void +static bool quiet_hook(const char *newval) { - pset.quiet = ParseVariableBool(newval, "QUIET"); + return generic_boolean_hook(newval, "QUIET", &pset.quiet); } -static void +static bool singleline_hook(const char *newval) { - pset.singleline = ParseVariableBool(newval, "SINGLELINE"); + return generic_boolean_hook(newval, "SINGLELINE", &pset.singleline); } -static void +static bool singlestep_hook(const char *newval) { - pset.singlestep = ParseVariableBool(newval, "SINGLESTEP"); + return generic_boolean_hook(newval, "SINGLESTEP", &pset.singlestep); } -static void +static bool fetch_count_hook(const char *newval) { pset.fetch_count = ParseVariableNum(newval, -1, -1, false); + return true; } -static void +static bool echo_hook(const char *newval) { if (newval == NULL) @@ -837,39 +853,52 @@ echo_hook(const char *newval) pset.echo = PSQL_ECHO_NONE; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "ECHO", "none"); - pset.echo = PSQL_ECHO_NONE; + psql_error("unrecognized value \"%s\" for \"%s\"\n", + newval, "ECHO"); + return false; } + return true; } -static void +static bool echo_hidden_hook(const char *newval) { if (newval == NULL) pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; else if (pg_strcasecmp(newval, "noexec") == 0) pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC; - else if (ParseVariableBool(newval, "ECHO_HIDDEN")) - pset.echo_hidden = PSQL_ECHO_HIDDEN_ON; - else /* ParseVariableBool printed msg if needed */ - pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; + else + { + bool isvalid; + bool val = ParseVariableBool(newval, "ECHO_HIDDEN", &isvalid); + if (!isvalid) + return false; /* ParseVariableBool printed msg */ + pset.echo_hidden = val ? PSQL_ECHO_HIDDEN_ON : PSQL_ECHO_HIDDEN_OFF; + } + return true; } -static void +static bool on_error_rollback_hook(const char *newval) { if (newval == NULL) pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF; else if (pg_strcasecmp(newval, "interactive") == 0) pset.on_error_rollback = PSQL_ERROR_ROLLBACK_INTERACTIVE; - else if (ParseVariableBool(newval, "ON_ERROR_ROLLBACK")) - pset.on_error_rollback = PSQL_ERROR_ROLLBACK_ON; - else /* ParseVariableBool printed msg if needed */ - pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF; + else + { + bool isvalid; + bool val = ParseVariableBool(newval, "ON_ERROR_ROLLBACK", &isvalid); + if (isvalid) + pset.on_error_rollback = val ? PSQL_ERROR_ROLLBACK_ON : PSQL_ERROR_ROLLBACK_OFF; + else + /* ParseVariableBool printed msg if needed */ + return false; + } + return true; } -static void +static bool comp_keyword_case_hook(const char *newval) { if (newval == NULL) @@ -884,13 +913,14 @@ comp_keyword_case_hook(const char *newval) pset.comp_case = PSQL_COMP_CASE_LOWER; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "COMP_KEYWORD_CASE", "preserve-upper"); - pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER; + psql_error("unrecognized value \"%s\" for \"%s\"\n", + newval, "COMP_KEYWORD_CASE"); + return false; } + return true; } -static void +static bool histcontrol_hook(const char *newval) { if (newval == NULL) @@ -905,31 +935,35 @@ histcontrol_hook(const char *newval) pset.histcontrol = hctl_none; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "HISTCONTROL", "none"); - pset.histcontrol = hctl_none; + psql_error("unrecognized value \"%s\" for \"%s\"\n", + newval, "HISTCONTROL"); + return false; } + return true; } -static void +static bool prompt1_hook(const char *newval) { pset.prompt1 = newval ? newval : ""; + return true; } -static void +static bool prompt2_hook(const char *newval) { pset.prompt2 = newval ? newval : ""; + return true; } -static void +static bool prompt3_hook(const char *newval) { pset.prompt3 = newval ? newval : ""; + return true; } -static void +static bool verbosity_hook(const char *newval) { if (newval == NULL) @@ -942,16 +976,17 @@ verbosity_hook(const char *newval) pset.verbosity = PQERRORS_VERBOSE; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "VERBOSITY", "default"); - pset.verbosity = PQERRORS_DEFAULT; + psql_error("unrecognized value \"%s\" for \"%s\"\n", + newval, "VERBOSITY"); + return false; } if (pset.db) PQsetErrorVerbosity(pset.db, pset.verbosity); + return true; } -static void +static bool show_context_hook(const char *newval) { if (newval == NULL) @@ -964,13 +999,14 @@ show_context_hook(const char *newval) pset.show_context = PQSHOW_CONTEXT_ALWAYS; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "SHOW_CONTEXT", "errors"); - pset.show_context = PQSHOW_CONTEXT_ERRORS; + psql_error("unrecognized value \"%s\" for \"%s\"\n", + newval, "SHOW_CONTEXT"); + return false; } if (pset.db) PQsetErrorContextVisibility(pset.db, pset.show_context); + return true; } diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c index f43f418..24ca750 100644 --- a/src/bin/psql/variables.c +++ b/src/bin/psql/variables.c @@ -86,12 +86,16 @@ GetVariable(VariableSpace space, const char *name) * * "name" is the name of the variable we're assigning to, to use in error * report if any. Pass name == NULL to suppress the error report. + * + * "*valid" reports whether "value" was syntactically valid, unless valid == NULL */ bool -ParseVariableBool(const char *value, const char *name) +ParseVariableBool(const char *value, const char *name, bool *valid) { size_t len; + if (valid) + *valid = true; if (value == NULL) return false; /* not set -> assume "off" */ @@ -116,10 +120,16 @@ ParseVariableBool(const char *value, const char *name) return false; else { - /* NULL is treated as false, so a non-matching value is 'true' */ + /* + * NULL is treated as false, so a non-matching value is 'true'. + * A caller that cares about syntactic conformance should + * check *valid to know whether the value was recognized. + */ if (name) - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - value, name, "on"); + psql_error("unrecognized value \"%s\" for \"%s\": boolean expected\n", + value, name); + if (valid) + *valid = false; return true; } } @@ -205,13 +215,19 @@ SetVariable(VariableSpace space, const char *name, const char *value) { if (strcmp(current->name, name) == 0) { - /* found entry, so update */ - if (current->value) - free(current->value); - current->value = pg_strdup(value); + /* found entry, so update, unless a hook returns false */ + bool confirmed = true; if (current->assign_hook) - (*current->assign_hook) (current->value); - return true; + { + confirmed = (*current->assign_hook) (value); + } + if (confirmed) + { + if (current->value) + free(current->value); + current->value = pg_strdup(value); + } + return confirmed; } } @@ -248,7 +264,7 @@ SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook { /* found entry, so update */ current->assign_hook = hook; - (*hook) (current->value); + (void)(*hook) (current->value); /* ignore return value */ return true; } } @@ -260,7 +276,7 @@ SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook current->assign_hook = hook; current->next = NULL; previous->next = current; - (*hook) (NULL); + (void)(*hook) (NULL); /* ignore return value */ return true; } diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h index d7a05a1..9836fc5 100644 --- a/src/bin/psql/variables.h +++ b/src/bin/psql/variables.h @@ -20,7 +20,7 @@ * Note: if value == NULL then the variable is logically unset, but we are * keeping the struct around so as not to forget about its hook function. */ -typedef void (*VariableAssignHook) (const char *newval); +typedef bool (*VariableAssignHook) (const char *newval); struct _variable { @@ -35,7 +35,7 @@ typedef struct _variable *VariableSpace; VariableSpace CreateVariableSpace(void); const char *GetVariable(VariableSpace space, const char *name); -bool ParseVariableBool(const char *value, const char *name); +bool ParseVariableBool(const char *value, const char *name, bool *valid); int ParseVariableNum(const char *val, int defaultval, int faultval,