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;