From 8e5f9c8160b5186c93c5d76789ffa88f3e5c2fde Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 9 Sep 2011 21:32:12 +0200 Subject: 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. --- src/conf.c | 21 +++++++------ src/conf.h | 18 +++++------ src/log.c | 16 +++++++--- src/main.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++-------------- 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 -- cgit v1.2.3