From a194cf56b3b31908e618d024a2886adcb0c9972b Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 4 Jul 2018 11:02:50 +0900 Subject: [PATCH] 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 + configfile.c | 14 +++++++-- configfile.h | 2 +- doc/appendix-release-notes.sgml | 9 ++++++ repmgr-client.c | 55 +++++++++++++++++++++++++++------ repmgrd.c | 2 +- 6 files changed, 69 insertions(+), 14 deletions(-) diff --git a/HISTORY b/HISTORY index 705b9083..4a7e8efc 100644 --- a/HISTORY +++ b/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) diff --git a/configfile.c b/configfile.c index 3690d053..c4a11289 100644 --- a/configfile.c +++ b/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); } diff --git a/configfile.h b/configfile.h index f10eba12..1d729e4b 100644 --- a/configfile.h +++ b/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_ */ diff --git a/doc/appendix-release-notes.sgml b/doc/appendix-release-notes.sgml index fb9d5354..591fea39 100644 --- a/doc/appendix-release-notes.sgml +++ b/doc/appendix-release-notes.sgml @@ -38,6 +38,15 @@ + + + repmgr: always exit with an error if an unrecognised + command line option is provided. This matches the behaviour of other PostgreSQL + utilities such as psql. (GitHub #464). + + + + repmgrd: create a PID file by default diff --git a/repmgr-client.c b/repmgr-client.c index 4984b201..3bd82b64 100644 --- a/repmgr-client.c +++ b/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 diff --git a/repmgrd.c b/repmgrd.c index dfad99f2..457dc916 100644 --- a/repmgrd.c +++ b/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;