repmgr: exit with an error if an unrecognised command line option is provided.

This matches the behaviour of other PostgreSQL utilities such as psql, though
repmgr will only abort once all command line options are parsed, so as many
errors as possible are found and displayed. If a repmgr "command" (e.g.
"repmgr primary ..." was provided, a hint about the relevant command
help section (e.g. "repmgr primary --help") will be provided alongside
the generic help command (i.e. "repmgr --help").

Addresses GitHub #464, with further improvements.

HISTORY

@@ -1,5 +1,6 @@
 4.1.0   2018-??-??
 		repmgr: add "--missing-slots" check to "repmgr node check" (Ian)
+        repmgr: improve command line error handling; GitHub #464 (Ian)
         repmgrd: create a PID file by default; GitHub #457 (Ian)
 		repmgrd: daemonize process by default; GitHub #458 (Ian)
 

configfile.c

@@ -1367,13 +1367,23 @@ exit_with_config_file_errors(ItemList *config_errors, ItemList *config_warnings,
 
 
 void
-exit_with_cli_errors(ItemList *error_list)
+exit_with_cli_errors(ItemList *error_list, const char *repmgr_command)
 {
 	fprintf(stderr, _("The following command line errors were encountered:\n"));
 
 	print_item_list(error_list);
 
-	fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname());
+	if (repmgr_command != NULL)
+	{
+		fprintf(stderr, _("Try \"%s --help\" or \"%s %s --help\" for more information.\n"),
+				progname(),
+				progname(),
+				repmgr_command);
+	}
+	else
+	{
+		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname());
+	}
 
 	exit(ERR_BAD_CONFIG);
 }

configfile.h

@@ -302,7 +302,7 @@ void free_parsed_argv(char ***argv_array);
 
 
 /* called by repmgr-client and repmgrd */
-void		exit_with_cli_errors(ItemList *error_list);
+void		exit_with_cli_errors(ItemList *error_list, const char *repmgr_command);
 void		print_item_list(ItemList *item_list);
 
 #endif							/* _REPMGR_CONFIGFILE_H_ */

doc/appendix-release-notes.sgml

@@ -38,6 +38,15 @@
       <para>
         <itemizedlist>
 
+          <listitem>
+            <para>
+              <application>repmgr</application>: always exit with an error if an unrecognised
+              command line option is provided. This matches the behaviour of other PostgreSQL
+              utilities such as <application>psql</application>. (GitHub #464).
+            </para>
+          </listitem>
+
+
           <listitem>
             <para>
               <application>repmgrd</application>: create a PID file by default

repmgr-client.c

@@ -98,7 +98,7 @@ main(int argc, char **argv)
 {
 	t_conninfo_param_list default_conninfo = T_CONNINFO_PARAM_LIST_INITIALIZER;
 
-	int			optindex;
+	int			optindex = 0;
 	int			c;
 
 	char	   *repmgr_command = NULL;
@@ -108,6 +108,7 @@ main(int argc, char **argv)
 	char	   *dummy_action = "";
 
 	bool		help_option = false;
+	bool		option_error_found = false;
 
 	set_progname(argv[0]);
 
@@ -178,6 +179,9 @@ main(int argc, char **argv)
 		strncpy(runtime_options.username, pw->pw_name, MAXLEN);
 	}
 
+	/* Make getopt emitting errors */
+	opterr = 1;
+
 	while ((c = getopt_long(argc, argv, "?Vb:f:FwWd:h:p:U:R:S:D:ck:L:tvC:", long_options,
 							&optindex)) != -1)
 	{
@@ -196,13 +200,7 @@ main(int argc, char **argv)
 			case OPT_HELP:		/* --help */
 				help_option = true;
 				break;
-			case '?':
-				/* Actual help option given */
-				if (strcmp(argv[optind - 1], "-?") == 0)
-				{
-					help_option = true;
-				}
-				break;
+
 			case 'V':
 
 				/*
@@ -631,9 +629,24 @@ main(int argc, char **argv)
 								 _("--recovery-min-apply-delay is now a configuration file parameter, \"recovery_min_apply_delay\""));
 				break;
 
+			case ':':   /* missing option argument */
+				option_error_found = true;
+				break;
+			case '?':
+				/* Actual help option given? */
+				if (strcmp(argv[optind - 1], "-?") == 0)
+				{
+					help_option = true;
+					break;
+				}
+				/* otherwise fall through to default */
+			default:    /* invalid option */
+				option_error_found = true;
+				break;
 		}
 	}
 
+
 	/*
 	 * If -d/--dbname appears to be a conninfo string, validate by attempting
 	 * to parse it (and if successful, store the parsed parameters)
@@ -734,9 +747,10 @@ main(int argc, char **argv)
 	if (cli_errors.head != NULL)
 	{
 		free_conninfo_params(&source_conninfo);
-		exit_with_cli_errors(&cli_errors);
+		exit_with_cli_errors(&cli_errors, NULL);
 	}
 
+
 	/*----------
 	 * Determine the node type and action; following are valid:
 	 *
@@ -983,9 +997,30 @@ main(int argc, char **argv)
 	if (cli_errors.head != NULL)
 	{
 		free_conninfo_params(&source_conninfo);
-		exit_with_cli_errors(&cli_errors);
+
+		exit_with_cli_errors(&cli_errors, valid_repmgr_command_found == true ? repmgr_command : NULL);
 	}
 
+	/* no errors detected by repmgr, but getopt might have */
+	if (option_error_found == true)
+	{
+		if (valid_repmgr_command_found == true)
+		{
+			printf(_("Try \"%s --help\" or \"%s %s --help\" for more information.\n"),
+				   progname(),
+				   progname(),
+				   repmgr_command);
+		}
+		else
+		{
+			printf(_("Try \"repmgr --help\" for more information.\n"));
+		}
+
+		free_conninfo_params(&source_conninfo);
+		exit(ERR_BAD_CONFIG);
+	}
+
+
 	/*
 	 * Print any warnings about inappropriate command line options, unless
 	 * -t/--terse set

repmgrd.c

@@ -237,7 +237,7 @@ main(int argc, char **argv)
 	/* Exit here already if errors in command line options found */
 	if (cli_errors.head != NULL)
 	{
-		exit_with_cli_errors(&cli_errors);
+		exit_with_cli_errors(&cli_errors, NULL);
 	}
 
 	startup_event_logged = false;