Make psql reject attempts to set special variables to invalid values.

Previously, if the user set a special variable such as ECHO to an
unrecognized value, psql would bleat but store the new value anyway, and
then fall back to a default setting for the behavior controlled by the
variable.  This was agreed to be a not particularly good idea.  With
this patch, invalid values result in an error message and no change in
state.

(But this applies only to variables that affect psql's behavior; purely
informational variables such as ENCODING can still be set to random
values.)

To do this, modify the API for psql's assign-hook functions so that they
can return an OK/not OK result, and give them the responsibility for
printing error messages when they reject a value.  Adjust the APIs for
ParseVariableBool and ParseVariableNum to support the new behavior
conveniently.

In passing, document the variable VERSION, which had somehow escaped that.
And improve the quite-inadequate commenting in psql/variables.c.

Daniel Vérité, reviewed by Rahila Syed, some further tweaking by me

Discussion: https://postgr.es/m/7356e741-fa59-4146-a8eb-cf95fd6b21fb@mm

1 files changed, 34 insertions, 16 deletions

diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index d235b1798e4..274b4af5537 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -3,25 +3,39 @@
  *
  * Copyright (c) 2000-2017, PostgreSQL Global Development Group
  *
+ * This implements a sort of variable repository.  One could also think of it
+ * as a cheap version of an associative array.  Each variable has a string
+ * name and a string value.  The value can't be NULL, or more precisely
+ * that's not distinguishable from the variable being unset.
+ *
  * src/bin/psql/variables.h
  */
 #ifndef VARIABLES_H
 #define VARIABLES_H
 
 /*
- * This implements a sort of variable repository. One could also think of it
- * as a cheap version of an associative array. In each one of these
- * datastructures you can store name/value pairs.  There can also be an
- * "assign hook" function that is called whenever the variable's value is
- * changed.
+ * Variables can be given "assign hook" functions.  The assign hook can
+ * prevent invalid values from being assigned, and can update internal C
+ * variables to keep them in sync with the variable's current value.
+ *
+ * A hook function is called before any attempted assignment, with the
+ * proposed new value of the variable (or with NULL, if an \unset is being
+ * attempted).  If it returns false, the assignment doesn't occur --- it
+ * should print an error message with psql_error() to tell the user why.
  *
- * An "unset" operation causes the hook to be called with newval == NULL.
+ * When a hook function is installed with SetVariableAssignHook(), it is
+ * called with the variable's current value (or with NULL, if it wasn't set
+ * yet).  But its return value is ignored in this case.  The hook should be
+ * set before any possibly-invalid value can be assigned.
+ */
+typedef bool (*VariableAssignHook) (const char *newval);
+
+/*
+ * Data structure representing one variable.
  *
  * 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);
-
 struct _variable
 {
 	char	   *name;
@@ -30,27 +44,31 @@ struct _variable
 	struct _variable *next;
 };
 
+/* Data structure representing a set of variables */
 typedef struct _variable *VariableSpace;
 
+
 VariableSpace CreateVariableSpace(void);
 const char *GetVariable(VariableSpace space, const char *name);
 
-bool		ParseVariableBool(const char *value, const char *name);
-int ParseVariableNum(const char *val,
-				 int defaultval,
-				 int faultval,
-				 bool allowtrail);
+bool ParseVariableBool(const char *value, const char *name,
+				  bool *result);
+
+bool ParseVariableNum(const char *value, const char *name,
+				 int *result);
+
 int GetVariableNum(VariableSpace space,
 			   const char *name,
 			   int defaultval,
-			   int faultval,
-			   bool allowtrail);
+			   int faultval);
 
 void		PrintVariables(VariableSpace space);
 
 bool		SetVariable(VariableSpace space, const char *name, const char *value);
-bool		SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
+void		SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
 bool		SetVariableBool(VariableSpace space, const char *name);
 bool		DeleteVariable(VariableSpace space, const char *name);
 
+void		PsqlVarEnumError(const char *name, const char *value, const char *suggestions);
+
 #endif   /* VARIABLES_H */