From: Andre Noll <maan@systemlinux.org>
Date: Fri, 28 Aug 2009 09:28:57 +0000 (+0200)
Subject: Improve error diagnostics.
X-Git-Tag: v0.1.4~19
X-Git-Url: http://git.tuebingen.mpg.de/?p=dss.git;a=commitdiff_plain;h=1e77c93a4da4dae8b75fcd305552e56ccae89b90;hp=7c487767f42a70358eea15eb9a86ac673856f905

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.
---

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, &params); /* aborts on errors */
+	}
 	if (conf.daemon_given)
 		daemon_init();
 	ret = change_to_dest_dir();