From 79706fd386ac29f85b13e906b55b70fe2d4b5ae2 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Fri, 8 Mar 2024 23:53:24 +0000 Subject: [PATCH] Minor improvements to options error handling - Add additional MTRs for more coverage on invalid options - Updating a few error messages to be more informative - Use the exit code from handle_options() when there is an error processing user options All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc. --- mysql-test/main/mysqld_option_err.result | 4 +++- mysql-test/main/mysqld_option_err.test | 14 ++++++++----- mysys/my_getopt.c | 26 +++++++++++++----------- sql/mysqld.cc | 5 +++-- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/mysql-test/main/mysqld_option_err.result b/mysql-test/main/mysqld_option_err.result index 2ad24ffe195..e740cb7631b 100644 --- a/mysql-test/main/mysqld_option_err.result +++ b/mysql-test/main/mysqld_option_err.result @@ -11,8 +11,10 @@ Test to see if multiple ambiguous options and invalid arguments will be displaye FOUND 1 /Error while setting value 'invalid_value' to 'sql_mode'/ in mysqltest.log FOUND 1 /ambiguous option '--character'/ in mysqltest.log FOUND 1 /option '--bootstrap' cannot take an argument/ in mysqltest.log -FOUND 1 /Incorrect integer value: '18446744073709551616'/ in mysqltest.log +FOUND 1 /Integer value out of range for uint64: '18446744073709551616'/ in mysqltest.log FOUND 1 /Error while setting value '18446744073709551616' to 'binlog_cache_size'/ in mysqltest.log +FOUND 1 /Unknown suffix 'y' used for variable 'bulk_insert_buffer_size' \(value '123y'\). Legal suffix characters are: K, M, G, T, P, E/ in mysqltest.log +FOUND 1 /Error while setting value '123y' to 'bulk_insert_buffer_size'/ in mysqltest.log Test that --help --verbose works Test that --not-known-option --help --verbose gives error Done. diff --git a/mysql-test/main/mysqld_option_err.test b/mysql-test/main/mysqld_option_err.test index e89eba33199..88c53794f53 100644 --- a/mysql-test/main/mysqld_option_err.test +++ b/mysql-test/main/mysqld_option_err.test @@ -25,7 +25,7 @@ mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err; --echo Test bad binlog format. ---error 1 +--error 13 --exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --log-bin --binlog-format=badformat >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1 @@ -35,7 +35,7 @@ mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err; --echo Test non-numeric value passed to number option. ---error 1 +--error 9 --exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --min-examined-row-limit=notanumber >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1 @@ -60,8 +60,8 @@ mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err; --source include/search_pattern_in_file.inc --echo Test to see if multiple ambiguous options and invalid arguments will be displayed in the error output ---error 1 ---exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --getopt-prefix-matching --sql-mode=invalid_value --character --bootstrap=partstoob --binlog_cache_size=18446744073709551616 >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1 +--error 9 +--exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --getopt-prefix-matching --sql-mode=invalid_value --character --bootstrap=partstoob --binlog_cache_size=18446744073709551616 --bulk_insert_buffer_size=123y >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1 --let SEARCH_PATTERN=Error while setting value 'invalid_value' to 'sql_mode' --source include/search_pattern_in_file.inc @@ -69,10 +69,14 @@ mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err; --source include/search_pattern_in_file.inc --let SEARCH_PATTERN=option '--bootstrap' cannot take an argument --source include/search_pattern_in_file.inc ---let SEARCH_PATTERN=Incorrect integer value: '18446744073709551616' +--let SEARCH_PATTERN=Integer value out of range for uint64: '18446744073709551616' --source include/search_pattern_in_file.inc --let SEARCH_PATTERN=Error while setting value '18446744073709551616' to 'binlog_cache_size' --source include/search_pattern_in_file.inc +--let SEARCH_PATTERN=Unknown suffix 'y' used for variable 'bulk_insert_buffer_size' \(value '123y'\). Legal suffix characters are: K, M, G, T, P, E +--source include/search_pattern_in_file.inc +--let SEARCH_PATTERN=Error while setting value '123y' to 'bulk_insert_buffer_size' +--source include/search_pattern_in_file.inc # # Test that an wrong option with --help --verbose gives an error diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c index e67407ca188..bed80d3efe2 100644 --- a/mysys/my_getopt.c +++ b/mysys/my_getopt.c @@ -858,7 +858,7 @@ static int setval(const struct my_option *opts, void *value, char *argument, } if (err) { - res= EXIT_UNKNOWN_SUFFIX; + res= err; goto ret; }; } @@ -992,7 +992,7 @@ static inline ulonglong eval_num_suffix(char *suffix, int *error) case 'E': return 1ULL << 60; default: - *error= 1; + *error= EXIT_UNKNOWN_SUFFIX; return 0ULL; } } @@ -1018,15 +1018,16 @@ static longlong eval_num_suffix_ll(char *argument, if (errno == ERANGE) { my_getopt_error_reporter(ERROR_LEVEL, - "Incorrect integer value: '%s'", argument); - *error= 1; + "Integer value out of range for int64: '%s'", argument); + *error= EXIT_ARGUMENT_INVALID; DBUG_RETURN(0); } num*= eval_num_suffix(endchar, error); if (*error) - fprintf(stderr, - "Unknown suffix '%c' used for variable '%s' (value '%s')\n", - *endchar, option_name, argument); + my_getopt_error_reporter(ERROR_LEVEL, + "Unknown suffix '%c' used for variable '%s' (value '%s'). " + "Legal suffix characters are: K, M, G, T, P, E", + *endchar, option_name, argument); DBUG_RETURN(num); } @@ -1050,15 +1051,16 @@ static ulonglong eval_num_suffix_ull(char *argument, if (errno == ERANGE) { my_getopt_error_reporter(ERROR_LEVEL, - "Incorrect integer value: '%s'", argument); - *error= 1; + "Integer value out of range for uint64: '%s'", argument); + *error= EXIT_ARGUMENT_INVALID; DBUG_RETURN(0); } num*= eval_num_suffix(endchar, error); if (*error) - fprintf(stderr, - "Unknown suffix '%c' used for variable '%s' (value '%s')\n", - *endchar, option_name, argument); + my_getopt_error_reporter(ERROR_LEVEL, + "Unknown suffix '%c' used for variable '%s' (value '%s'). " + "Legal suffix characters are: K, M, G, T, P, E", + *endchar, option_name, argument); DBUG_RETURN(num); } diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 5ba51bdfb74..128899cb347 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -3860,8 +3860,9 @@ static int init_common_variables() SQLCOM_END + 11); #endif - if (get_options(&remaining_argc, &remaining_argv)) - exit(1); + int opt_err; + if ((opt_err= get_options(&remaining_argc, &remaining_argv))) + exit(opt_err); if (IS_SYSVAR_AUTOSIZE(&server_version_ptr)) set_server_version(server_version, sizeof(server_version));