conf: handle fatal errors with GError

Don't call g_error(), which will abort the process and dump core.

This patch does not affect all the config_get_X() functions.  These
need some more refactoring.

7 files changed, 136 insertions, 58 deletions

diff --git a/src/cmdline.c b/src/cmdline.c
index 44b3ff50..9e391915 100644
--- a/src/cmdline.c
+++ b/src/cmdline.c
@@ -152,21 +152,20 @@ parse_cmdline(int argc, char **argv, struct options *options,
 		path2 = g_build_filename(g_get_home_dir(),
 					USER_CONFIG_FILE_LOCATION2, NULL);
 		if (g_file_test(path1, G_FILE_TEST_IS_REGULAR))
-			config_read_file(path1);
+			ret = config_read_file(path1, error_r);
 		else if (g_file_test(path2, G_FILE_TEST_IS_REGULAR))
-			config_read_file(path2);
+			ret = config_read_file(path2, error_r);
 		else if (g_file_test(SYSTEM_CONFIG_FILE_LOCATION,
 				     G_FILE_TEST_IS_REGULAR))
-			config_read_file(SYSTEM_CONFIG_FILE_LOCATION);
+			ret = config_read_file(SYSTEM_CONFIG_FILE_LOCATION,
+					       error_r);
 		g_free(path1);
 		g_free(path2);
 
-		return true;
+		return ret;
 	} else if (argc == 2) {
 		/* specified configuration file */
-		config_read_file(argv[1]);
-
-		return true;
+		return config_read_file(argv[1], error_r);
 	} else {
 		g_set_error(error_r, cmdline_quark(), 0,
 			    "too many arguments");
diff --git a/src/conf.c b/src/conf.c
index fe4b39fc..d78f4476 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -221,17 +221,20 @@ void config_global_check(void)
 	}
 }
 
-void
+bool
 config_add_block_param(struct config_param * param, const char *name,
-		       const char *value, int line)
+		       const char *value, int line, GError **error_r)
 {
 	struct block_param *bp;
 
 	bp = config_get_block_param(param, name);
-	if (bp != NULL)
-		g_error("\"%s\" first defined on line %i, and "
-			"redefined on line %i\n", name,
-			bp->line, line);
+	if (bp != NULL) {
+		g_set_error(error_r, config_quark(), 0,
+			    "\"%s\" first defined on line %i, and "
+			    "redefined on line %i\n", name,
+			    bp->line, line);
+		return false;
+	}
 
 	param->num_block_params++;
 
@@ -245,21 +248,28 @@ config_add_block_param(struct config_param * param, const char *name,
 	bp->value = g_strdup(value);
 	bp->line = line;
 	bp->used = false;
+
+	return true;
 }
 
 static struct config_param *
-config_read_block(FILE *fp, int *count, char *string)
+config_read_block(FILE *fp, int *count, char *string, GError **error_r)
 {
 	struct config_param *ret = config_new_param(NULL, *count);
+	GError *error = NULL;
+	bool success;
 
 	while (true) {
 		char *line;
 		const char *name, *value;
-		GError *error = NULL;
 
 		line = fgets(string, MAX_STRING_SIZE, fp);
-		if (line == NULL)
-			g_error("Expected '}' before end-of-file");
+		if (line == NULL) {
+			config_param_free(ret);
+			g_set_error(error_r, config_quark(), 0,
+				    "Expected '}' before end-of-file");
+			return NULL;
+		}
 
 		(*count)++;
 		line = g_strchug(line);
@@ -271,9 +281,13 @@ config_read_block(FILE *fp, int *count, char *string)
 			   (and from this "while" loop) */
 
 			line = g_strchug(line + 1);
-			if (*line != 0 && *line != CONF_COMMENT)
-				g_error("line %i: Unknown tokens after '}'",
-					*count);
+			if (*line != 0 && *line != CONF_COMMENT) {
+				config_param_free(ret);
+				g_set_error(error_r, config_quark(), 0,
+					    "line %i: Unknown tokens after '}'",
+					    *count);
+				return false;
+			}
 
 			return ret;
 		}
@@ -283,25 +297,44 @@ config_read_block(FILE *fp, int *count, char *string)
 		name = tokenizer_next_word(&line, &error);
 		if (name == NULL) {
 			assert(*line != 0);
-			g_error("line %i: %s", *count, error->message);
+			config_param_free(ret);
+			g_propagate_prefixed_error(error_r, error,
+						   "line %i: ", *count);
+			return NULL;
 		}
 
 		value = tokenizer_next_string(&line, &error);
 		if (value == NULL) {
+			config_param_free(ret);
 			if (*line == 0)
-				g_error("line %i: Value missing", *count);
+				g_set_error(error_r, config_quark(), 0,
+					    "line %i: Value missing", *count);
 			else
-				g_error("line %i: %s", *count, error->message);
+				g_propagate_prefixed_error(error_r, error,
+							   "line %i: ",
+							   *count);
+			return NULL;
 		}
 
-		if (*line != 0 && *line != CONF_COMMENT)
-			g_error("line %i: Unknown tokens after value", *count);
+		if (*line != 0 && *line != CONF_COMMENT) {
+			config_param_free(ret);
+			g_set_error(error_r, config_quark(), 0,
+				    "line %i: Unknown tokens after value",
+				    *count);
+			return NULL;
+		}
 
-		config_add_block_param(ret, name, value, *count);
+		success = config_add_block_param(ret, name, value, *count,
+						 error_r);
+		if (!success) {
+			config_param_free(ret);
+			return false;
+		}
 	}
 }
 
-void config_read_file(const char *file)
+bool
+config_read_file(const char *file, GError **error_r)
 {
 	FILE *fp;
 	char string[MAX_STRING_SIZE + 1];
@@ -312,8 +345,10 @@ void config_read_file(const char *file)
 	g_debug("loading file %s", file);
 
 	if (!(fp = fopen(file, "r"))) {
-		g_error("problems opening file %s for reading: %s\n",
-			file, strerror(errno));
+		g_set_error(error_r, config_quark(), errno,
+			    "Failed to open %s: %s",
+			    file, strerror(errno));
+		return false;
 	}
 
 	while (fgets(string, MAX_STRING_SIZE, fp)) {
@@ -333,22 +368,29 @@ void config_read_file(const char *file)
 		name = tokenizer_next_word(&line, &error);
 		if (name == NULL) {
 			assert(*line != 0);
-			g_error("line %i: %s", count, error->message);
+			g_propagate_prefixed_error(error_r, error,
+						   "line %i: ", count);
+			return false;
 		}
 
 		/* get the definition of that option, and check the
 		   "repeatable" flag */
 
 		entry = config_entry_get(name);
-		if (entry == NULL)
-			g_error("unrecognized parameter in config file at "
-				"line %i: %s\n", count, name);
+		if (entry == NULL) {
+			g_set_error(error_r, config_quark(), 0,
+				    "unrecognized parameter in config file at "
+				    "line %i: %s\n", count, name);
+			return false;
+		}
 
 		if (entry->params != NULL && !entry->repeatable) {
 			param = entry->params->data;
-			g_error("config parameter \"%s\" is first defined on "
-				"line %i and redefined on line %i\n",
-				name, param->line, count);
+			g_set_error(error_r, config_quark(), 0,
+				    "config parameter \"%s\" is first defined "
+				    "on line %i and redefined on line %i\n",
+				    name, param->line, count);
+			return false;
 		}
 
 		/* now parse the block or the value */
@@ -356,31 +398,48 @@ void config_read_file(const char *file)
 		if (entry->block) {
 			/* it's a block, call config_read_block() */
 
-			if (*line != '{')
-				g_error("line %i: '{' expected", count);
+			if (*line != '{') {
+				g_set_error(error_r, config_quark(), 0,
+					    "line %i: '{' expected", count);
+				return false;
+			}
 
 			line = g_strchug(line + 1);
-			if (*line != 0 && *line != CONF_COMMENT)
-				g_error("line %i: Unknown tokens after '{'",
-					count);
+			if (*line != 0 && *line != CONF_COMMENT) {
+				g_set_error(error_r, config_quark(), 0,
+					    "line %i: Unknown tokens after '{'",
+					    count);
+				return false;
+			}
 
-			param = config_read_block(fp, &count, string);
+			param = config_read_block(fp, &count, string, error_r);
+			if (param == NULL)
+				return false;
 		} else {
 			/* a string value */
 
 			value = tokenizer_next_string(&line, &error);
 			if (value == NULL) {
 				if (*line == 0)
-					g_error("line %i: Value missing",
-						count);
-				else
-					g_error("line %i: %s", count,
-						error->message);
+					g_set_error(error_r, config_quark(), 0,
+						    "line %i: Value missing",
+						    count);
+				else {
+					g_set_error(error_r, config_quark(), 0,
+						    "line %i: %s", count,
+						    error->message);
+					g_error_free(error);
+				}
+
+				return false;
 			}
 
-			if (*line != 0 && *line != CONF_COMMENT)
-				g_error("line %i: Unknown tokens after value",
-					count);
+			if (*line != 0 && *line != CONF_COMMENT) {
+				g_set_error(error_r, config_quark(), 0,
+					    "line %i: Unknown tokens after value",
+					    count);
+				return false;
+			}
 
 			param = config_new_param(value, count);
 		}
@@ -388,6 +447,8 @@ void config_read_file(const char *file)
 		entry->params = g_slist_append(entry->params, param);
 	}
 	fclose(fp);
+
+	return true;
 }
 
 struct config_param *
diff --git a/src/conf.h b/src/conf.h
index a153b715..ef10b9f8 100644
--- a/src/conf.h
+++ b/src/conf.h
@@ -116,7 +116,8 @@ void config_global_finish(void);
  */
 void config_global_check(void);
 
-void config_read_file(const char *file);
+bool
+config_read_file(const char *file, GError **error_r);
 
 /* don't free the returned value
    set _last_ to NULL to get first entry */
@@ -192,8 +193,8 @@ config_get_block_bool(const struct config_param *param, const char *name,
 struct config_param *
 config_new_param(const char *value, int line);
 
-void
-config_add_block_param(struct config_param *param, const char *name,
-		       const char *value, int line);
+bool
+config_add_block_param(struct config_param * param, const char *name,
+		       const char *value, int line, GError **error_r);
 
 #endif
diff --git a/test/read_conf.c b/test/read_conf.c
index 286ec2c7..92fcbba9 100644
--- a/test/read_conf.c
+++ b/test/read_conf.c
@@ -37,6 +37,8 @@ my_log_func(G_GNUC_UNUSED const gchar *log_domain,
 int main(int argc, char **argv)
 {
 	const char *path, *name, *value;
+	GError *error = NULL;
+	bool success;
 	int ret;
 
 	if (argc != 3) {
@@ -50,7 +52,12 @@ int main(int argc, char **argv)
 	g_log_set_default_handler(my_log_func, NULL);
 
 	config_global_init();
-	config_read_file(path);
+	success = config_read_file(path, &error);
+	if (!success) {
+		g_printerr("%s:", error->message);
+		g_error_free(error);
+		return 1;
+	}
 
 	value = config_get_string(name, NULL);
 	if (value != NULL) {
diff --git a/test/run_encoder.c b/test/run_encoder.c
index a9b00e95..787fb03c 100644
--- a/test/run_encoder.c
+++ b/test/run_encoder.c
@@ -73,7 +73,7 @@ int main(int argc, char **argv)
 	}
 
 	param = config_new_param(NULL, -1);
-	config_add_block_param(param, "quality", "5.0", -1);
+	config_add_block_param(param, "quality", "5.0", -1, NULL);
 
 	encoder = encoder_init(plugin, param, &error);
 	if (encoder == NULL) {
diff --git a/test/run_filter.c b/test/run_filter.c
index 3c98491a..ed2b904c 100644
--- a/test/run_filter.c
+++ b/test/run_filter.c
@@ -90,7 +90,12 @@ int main(int argc, char **argv)
 	/* read configuration file (mpd.conf) */
 
 	config_global_init();
-	config_read_file(argv[1]);
+	success = config_read_file(argv[1], &error);
+	if (!success) {
+		g_printerr("%s:", error->message);
+		g_error_free(error);
+		return 1;
+	}
 
 	/* parse the audio format */
 
diff --git a/test/run_output.c b/test/run_output.c
index 594c4cd6..5ab9625e 100644
--- a/test/run_output.c
+++ b/test/run_output.c
@@ -122,7 +122,12 @@ int main(int argc, char **argv)
 	/* read configuration file (mpd.conf) */
 
 	config_global_init();
-	config_read_file(argv[1]);
+	success = config_read_file(argv[1], &error);
+	if (!success) {
+		g_printerr("%s:", error->message);
+		g_error_free(error);
+		return 1;
+	}
 
 	/* initialize the audio output */