From 1e77c93a4da4dae8b75fcd305552e56ccae89b90 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Fri, 28 Aug 2009 11:28:57 +0200 Subject: [PATCH] Improve error diagnostics. When parsing the command line options we must not error out if a required option was not given because that option might be specified in the config file. Therefore we have to call cmdline_parser_ext() with params->check_required = 0. However, if --config-file is not given and the default config file (~/.dssrc) does not exist, we end up with no check for required options at all. In particular, if the required --dest-dir option is not given, conf.dest_dir is NULL and we call chdir(NULL) which returns EBADADRESS at least on Linux. This causes dss to print the error message Aug 28 11:35:07 main: Bad address which is not really helpful. Fix this shortcoming by calling cmdline_parser_ext() _again_ if no config file was read by parse_config_file(). This second call uses params->check_required = 1, so that a proper error message is printed if any required options are missing. In the above example, the output changes to ./dss: '--source-dir' option required ./dss: '--dest-dir' option required which is much better. --- dss.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/dss.c b/dss.c index 04fe30c..a233c11 100644 --- a/dss.c +++ b/dss.c @@ -818,9 +818,13 @@ static int check_config(void) return 1; } +/* + * Returns < 0 on errors, 0 if no config file is given and > 0 if the config + * file was read successfully. + */ static int parse_config_file(int override) { - int ret; + int ret, config_file_exists; char *config_file; struct stat statbuf; char *old_logfile_arg = NULL; @@ -839,13 +843,13 @@ static int parse_config_file(int override) old_daemon_given = conf.daemon_given; } - ret = stat(config_file, &statbuf); - if (ret && conf.config_file_given) { + config_file_exists = !stat(config_file, &statbuf); + if (!config_file_exists && conf.config_file_given) { ret = -ERRNO_TO_DSS_ERROR(errno); DSS_ERROR_LOG("failed to stat config file %s\n", config_file); goto out; } - if (!ret) { + if (config_file_exists) { struct cmdline_parser_params params = { .override = override, .initialize = 0, @@ -885,6 +889,7 @@ static int parse_config_file(int override) } DSS_DEBUG_LOG("loglevel: %d\n", conf.loglevel_arg); // cmdline_parser_dump(logfile? logfile : stdout, &conf); + ret = config_file_exists; out: free(config_file); if (ret < 0) @@ -1294,6 +1299,20 @@ int main(int argc, char **argv) ret = parse_config_file(0); if (ret < 0) goto out; + if (ret == 0) { /* no config file given */ + /* + * Parse the command line options again, but this time check + * that all required options are given. + */ + params = (struct cmdline_parser_params) { + .override = 1, + .initialize = 1, + .check_required = 1, + .check_ambiguity = 1, + .print_errors = 1 + }; + cmdline_parser_ext(argc, argv, &conf, ¶ms); /* aborts on errors */ + } if (conf.daemon_given) daemon_init(); ret = change_to_dest_dir(); -- 2.39.5