aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kellermann <max@duempel.org>2011-09-09 21:32:12 +0200
committerMax Kellermann <max@duempel.org>2011-09-09 22:55:57 +0200
commit8e5f9c8160b5186c93c5d76789ffa88f3e5c2fde (patch)
treef71eb9f9c5dfa789f94eae4182490286b54788ad
parentc620fd42f4e8c36186c1e3c3523ac6bd1351f91d (diff)
downloadmpd-8e5f9c8160b5186c93c5d76789ffa88f3e5c2fde.tar.gz
mpd-8e5f9c8160b5186c93c5d76789ffa88f3e5c2fde.tar.xz
mpd-8e5f9c8160b5186c93c5d76789ffa88f3e5c2fde.zip
conf: turn config_get_path() into config_dup_path()
config_get_path() was somewhat flawed, because it pretended to be a function, when it really had a side effect. The second flaw was that it did not return the parser error, instead it aborted the whole process, which is bad style. The new function returns a duplicated (modified) string that must be freed by the caller, and returns a GError on failure.
-rw-r--r--src/conf.c21
-rw-r--r--src/conf.h18
-rw-r--r--src/log.c16
-rw-r--r--src/main.c101
4 files changed, 109 insertions, 47 deletions
diff --git a/src/conf.c b/src/conf.c
index 4434a875d..b3b684aa5 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -503,22 +503,23 @@ config_get_string(const char *name, const char *default_value)
return param->value;
}
-const char *
-config_get_path(const char *name)
+char *
+config_dup_path(const char *name, GError **error_r)
{
- struct config_param *param = config_get_param(name);
- char *path;
+ assert(error_r != NULL);
+ assert(*error_r == NULL);
+ const struct config_param *param = config_get_param(name);
if (param == NULL)
return NULL;
- path = parsePath(param->value);
- if (path == NULL)
- MPD_ERROR("error parsing \"%s\" at line %i\n",
- name, param->line);
+ char *path = parsePath(param->value);
+ if (G_UNLIKELY(path == NULL))
+ g_set_error(error_r, config_quark(), 0,
+ "Invalid path in \"%s\" at line %i",
+ name, param->line);
- g_free(param->value);
- return param->value = path;
+ return path;
}
unsigned
diff --git a/src/conf.h b/src/conf.h
index 8aad9839f..9593501b0 100644
--- a/src/conf.h
+++ b/src/conf.h
@@ -156,17 +156,15 @@ config_get_string(const char *name, const char *default_value);
/**
* Returns an optional configuration variable which contains an
- * absolute path. If there is a tilde prefix, it is expanded. Aborts
- * MPD if the path is not a valid absolute path.
+ * absolute path. If there is a tilde prefix, it is expanded.
+ * Returns NULL if the value is not present. If the path could not be
+ * parsed, returns NULL and sets the error.
+ *
+ * The return value must be freed with g_free().
*/
-/* We lie here really. This function is not pure as it has side
- effects -- it parse the value and creates new string freeing
- previous one. However, because this works the very same way each
- time (ie. from the outside it appears as if function had no side
- effects) we should be in the clear declaring it pure. */
-G_GNUC_PURE
-const char *
-config_get_path(const char *name);
+G_GNUC_MALLOC
+char *
+config_dup_path(const char *name, GError **error_r);
G_GNUC_PURE
unsigned
diff --git a/src/log.c b/src/log.c
index 140b50a2e..86dd86eaa 100644
--- a/src/log.c
+++ b/src/log.c
@@ -271,10 +271,18 @@ log_init(bool verbose, bool use_stdout, GError **error_r)
return true;
#endif
} else {
- const char *path = config_get_path(CONF_LOG_FILE);
- assert(path != NULL);
-
- return log_init_file(path, param->line, error_r);
+ GError *error = NULL;
+ char *path = config_dup_path(CONF_LOG_FILE, &error);
+ if (path == NULL) {
+ assert(error != NULL);
+ g_propagate_error(error_r, error);
+ return false;
+ }
+
+ bool success = log_init_file(path, param->line,
+ error_r);
+ g_free(path);
+ return success;
}
}
}
diff --git a/src/main.c b/src/main.c
index a80b4e17f..a57a19a81 100644
--- a/src/main.c
+++ b/src/main.c
@@ -98,31 +98,54 @@ GCond *main_cond;
struct player_control *global_player_control;
-static void
-glue_daemonize_init(const struct options *options)
+static bool
+glue_daemonize_init(const struct options *options, GError **error_r)
{
+ GError *error = NULL;
+
+ char *pid_file = config_dup_path(CONF_PID_FILE, &error);
+ if (pid_file == NULL && error != NULL) {
+ g_propagate_error(error_r, error);
+ return false;
+ }
+
daemonize_init(config_get_string(CONF_USER, NULL),
config_get_string(CONF_GROUP, NULL),
- config_get_path(CONF_PID_FILE));
+ pid_file);
+ g_free(pid_file);
if (options->kill)
daemonize_kill();
+
+ return true;
}
-static void
-glue_mapper_init(void)
+static bool
+glue_mapper_init(GError **error_r)
{
- const char *music_dir, *playlist_dir;
+ GError *error = NULL;
+ char *music_dir = config_dup_path(CONF_MUSIC_DIR, &error);
+ if (music_dir == NULL && error != NULL) {
+ g_propagate_error(error_r, error);
+ return false;
+ }
+
+ char *playlist_dir = config_dup_path(CONF_PLAYLIST_DIR, &error);
+ if (playlist_dir == NULL && error != NULL) {
+ g_propagate_error(error_r, error);
+ return false;
+ }
- music_dir = config_get_path(CONF_MUSIC_DIR);
#if GLIB_CHECK_VERSION(2,14,0)
if (music_dir == NULL)
- music_dir = g_get_user_special_dir(G_USER_DIRECTORY_MUSIC);
+ music_dir = g_strdup(g_get_user_special_dir(G_USER_DIRECTORY_MUSIC));
#endif
- playlist_dir = config_get_path(CONF_PLAYLIST_DIR);
-
mapper_init(music_dir, playlist_dir);
+
+ g_free(music_dir);
+ g_free(playlist_dir);
+ return true;
}
/**
@@ -133,11 +156,15 @@ glue_mapper_init(void)
static bool
glue_db_init_and_load(void)
{
- const char *path = config_get_path(CONF_DB_FILE);
- bool ret;
GError *error = NULL;
+ char *path = config_dup_path(CONF_DB_FILE, &error);
+ if (path == NULL && error != NULL)
+ MPD_ERROR("%s", error->message);
+
+ bool ret;
if (!mapper_has_music_directory()) {
+ g_free(path);
if (path != NULL)
g_message("Found " CONF_DB_FILE " setting without "
CONF_MUSIC_DIR " - disabling database");
@@ -149,6 +176,7 @@ glue_db_init_and_load(void)
MPD_ERROR(CONF_DB_FILE " setting missing");
db_init(path);
+ g_free(path);
ret = db_load(&error);
if (!ret) {
@@ -174,21 +202,34 @@ static void
glue_sticker_init(void)
{
#ifdef ENABLE_SQLITE
- bool success;
GError *error = NULL;
+ char *sticker_file = config_dup_path(CONF_STICKER_FILE, &error);
+ if (sticker_file == NULL && error != NULL)
+ MPD_ERROR("%s", error->message);
- success = sticker_global_init(config_get_path(CONF_STICKER_FILE),
- &error);
- if (!success)
+ if (!sticker_global_init(config_get_string(CONF_STICKER_FILE, NULL),
+ &error))
MPD_ERROR("%s", error->message);
+
+ g_free(sticker_file);
#endif
}
-static void
-glue_state_file_init(void)
+static bool
+glue_state_file_init(GError **error_r)
{
- state_file_init(config_get_path(CONF_STATE_FILE),
- global_player_control);
+ GError *error = NULL;
+
+ char *path = config_dup_path(CONF_STATE_FILE, &error);
+ if (path == NULL && error != NULL) {
+ g_propagate_error(error_r, error);
+ return false;
+ }
+
+ state_file_init(path, global_player_control);
+ g_free(path);
+
+ return true;
}
/**
@@ -328,7 +369,11 @@ int mpd_main(int argc, char *argv[])
return EXIT_FAILURE;
}
- glue_daemonize_init(&options);
+ if (!glue_daemonize_init(&options, &error)) {
+ g_warning("%s", error->message);
+ g_error_free(error);
+ return EXIT_FAILURE;
+ }
stats_global_init();
tag_lib_init();
@@ -357,7 +402,13 @@ int mpd_main(int argc, char *argv[])
event_pipe_register(PIPE_EVENT_SHUTDOWN, shutdown_event_emitted);
path_global_init();
- glue_mapper_init();
+
+ if (!glue_mapper_init(&error)) {
+ g_warning("%s", error->message);
+ g_error_free(error);
+ return EXIT_FAILURE;
+ }
+
initPermissions();
playlist_global_init();
spl_global_init();
@@ -411,7 +462,11 @@ int mpd_main(int argc, char *argv[])
MPD_ERROR("directory update failed");
}
- glue_state_file_init();
+ if (!glue_state_file_init(&error)) {
+ g_warning("%s", error->message);
+ g_error_free(error);
+ return EXIT_FAILURE;
+ }
success = config_get_bool(CONF_AUTO_UPDATE, false);
#ifdef ENABLE_INOTIFY