From 9ebea2976dff872a068f515ae7aa304052485970 Mon Sep 17 00:00:00 2001 From: George Kiagiadakis Date: Fri, 16 Jul 2021 19:21:00 +0300 Subject: [PATCH] wp: refactor and rename wp_iter_conf_files() to use WpIterator This also corrects the logic of loading config files in m-lua-scripting. Previously, if an error occured within the iteration function, the error was not properly propagated to the caller because -EINVAL was being added to nfiles instead of checked. --- lib/wp/wp.c | 126 ++++++++++++++++++-------- lib/wp/wp.h | 16 +--- modules/module-lua-scripting/config.c | 36 ++++++-- 3 files changed, 115 insertions(+), 63 deletions(-) diff --git a/lib/wp/wp.c b/lib/wp/wp.c index 5499c73b..b39258ab 100644 --- a/lib/wp/wp.c +++ b/lib/wp/wp.c @@ -293,40 +293,96 @@ wp_find_sysconfig_file (const gchar *filename, const char *subdir) return NULL; } +/*! \} */ + +struct conffile_iterator_data +{ + GList *sorted_keys; + GList *ptr; + GHashTable *ht; +}; + +static void +conffile_iterator_reset (WpIterator *it) +{ + struct conffile_iterator_data *it_data = wp_iterator_get_user_data (it); + it_data->ptr = it_data->sorted_keys; +} + +static gboolean +conffile_iterator_next (WpIterator *it, GValue *item) +{ + struct conffile_iterator_data *it_data = wp_iterator_get_user_data (it); + + if (it_data->ptr) { + const gchar *path = g_hash_table_lookup (it_data->ht, it_data->ptr->data); + it_data->ptr = g_list_next (it_data->ptr); + g_value_init (item, G_TYPE_STRING); + g_value_set_string (item, path); + return TRUE; + } + return FALSE; +} + +static gboolean +conffile_iterator_fold (WpIterator *it, WpIteratorFoldFunc func, GValue *ret, + gpointer data) +{ + struct conffile_iterator_data *it_data = wp_iterator_get_user_data (it); + + for (GList *ptr = it_data->sorted_keys; ptr != NULL; ptr = g_list_next (ptr)) { + g_auto (GValue) item = G_VALUE_INIT; + const gchar *path = g_hash_table_lookup (it_data->ht, ptr->data); + g_value_init (&item, G_TYPE_STRING); + g_value_set_string (&item, path); + if (!func (&item, ret, data)) + return FALSE; + } + return TRUE; +} + +static void +conffile_iterator_finalize (WpIterator *it) +{ + struct conffile_iterator_data *it_data = wp_iterator_get_user_data (it); + g_list_free (it_data->sorted_keys); + g_hash_table_unref (it_data->ht); +} + +static const WpIteratorMethods conffile_iterator_methods = { + .version = WP_ITERATOR_METHODS_VERSION, + .reset = conffile_iterator_reset, + .next = conffile_iterator_next, + .fold = conffile_iterator_fold, + .finalize = conffile_iterator_finalize, +}; + /*! - * \brief Iterates over configuration files in the \a subdir and calls the - * \a func for each file. + * \brief Creates an iterator to iterate over configuration files in the + * \a subdir of the configuration directories * * Files are sorted across the hierarchy of configuration and data * directories with files in higher-priority directories shadowing files in * lower-priority directories. Files are only checked for existence, a * caller must be able to handle read errors. * - * If the \a func returns a negative errno the iteration stops and that - * errno is returned to the caller. The \a func should set \a error on - * failure. - * - * \note \a func is called for directories too, it is the responsibility + * \note the iterator may contain directories too; it is the responsibility * of the caller to ignore or recurse into those. * + * \ingroup wp * \param subdir (nullable): the name of the subdirectory to search in, * inside the configuration directories * \param suffix (nullable): The filename suffix, NULL matches all entries - * \param func (scope call): The callback to invoke for each file. - * \param user_data (closure): Passed through to \a func - * \param error (out)(optional): Passed through to \a func - * \returns the number of files on success or a negative errno on failure + * \returns (transfer full): a new iterator iterating over strings which are + * absolute paths to the configuration files found * \since 0.4.2 */ -int -wp_iter_config_files (const gchar *subdir, const gchar *suffix, - wp_file_iter_func func, gpointer user_data, - GError **error) +WpIterator * +wp_new_config_files_iterator (const gchar *subdir, const gchar *suffix) { - g_autoptr(GHashTable) ht = g_hash_table_new_full (g_str_hash, g_str_equal, - g_free, g_free); - g_autoptr(GPtrArray) dirs = NULL; - gint count = 0; + g_autoptr (GHashTable) ht = + g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + g_autoptr (GPtrArray) dirs = NULL; if (subdir == NULL) subdir = "."; @@ -337,8 +393,8 @@ wp_iter_config_files (const gchar *subdir, const gchar *suffix, /* Store all filenames with their full path in the hashtable, overriding * previous values. We need to run backwards through the list for that */ for (guint i = dirs->len; i > 0; i--) { - g_autofree gchar *dirpath = g_build_filename (g_ptr_array_index (dirs, i - 1), - subdir, NULL); + g_autofree gchar *dirpath = + g_build_filename (g_ptr_array_index (dirs, i - 1), subdir, NULL); g_autoptr(GDir) dir = g_dir_open (dirpath, 0, NULL); wp_trace ("searching config dir: %s", dirpath); @@ -350,30 +406,20 @@ wp_iter_config_files (const gchar *subdir, const gchar *suffix, continue; g_hash_table_replace (ht, g_strdup (filename), - g_build_filename (dirpath, filename, NULL)); + g_build_filename (dirpath, filename, NULL)); } } } - if (g_hash_table_size (ht) == 0) - return 0; - /* Sort by filename */ - g_autoptr(GList) keys = g_hash_table_get_keys (ht); + GList *keys = g_hash_table_get_keys (ht); keys = g_list_sort (keys, (GCompareFunc)g_strcmp0); - /* Now we have our filenames in a sorted order so we can call the callback */ - for (GList *elem = g_list_first (keys); elem; elem = g_list_next (elem)) { - const gchar *path = g_hash_table_lookup (ht, elem->data); - gint rc = func (path, user_data, error); - - if (rc < 0) - return rc; - - count++; - } - - return count; + /* Construct iterator */ + WpIterator *it = wp_iterator_new (&conffile_iterator_methods, + sizeof (struct conffile_iterator_data)); + struct conffile_iterator_data *it_data = wp_iterator_get_user_data (it); + it_data->sorted_keys = keys; + it_data->ht = g_hash_table_ref (ht); + return g_steal_pointer (&it); } - -/*! \} */ diff --git a/lib/wp/wp.h b/lib/wp/wp.h index 411b39ee..5966e489 100644 --- a/lib/wp/wp.h +++ b/lib/wp/wp.h @@ -85,21 +85,9 @@ gchar * wp_find_config_file (const gchar *filename, const char *subdir); WP_API gchar * wp_find_sysconfig_file (const gchar *filename, const char *subdir); -/*! - * \brief A function to iterate over files - * \ingroup wp - * \param filename the absolute file path of the current file or directory - * \param user_data user data passed from the caller - * \param error (out)(optional): error to be set on failure - * \returns 0 on success, a negative errno on failure - */ -typedef gint (*wp_file_iter_func)(const gchar *filename, gpointer user_data, - GError **error); - WP_API -gint wp_iter_config_files (const gchar *subdir, const gchar *suffix, - wp_file_iter_func func, gpointer user_data, - GError **error); +WpIterator * wp_new_config_files_iterator (const gchar *subdir, + const gchar *suffix); G_END_DECLS diff --git a/modules/module-lua-scripting/config.c b/modules/module-lua-scripting/config.c index 0449384e..1b90d192 100644 --- a/modules/module-lua-scripting/config.c +++ b/modules/module-lua-scripting/config.c @@ -8,7 +8,6 @@ #include #include -#include static gboolean load_components (lua_State *L, WpCore * core, GError ** error) @@ -78,26 +77,36 @@ done: return TRUE; } -static gint -load_file (const gchar *path, gpointer data, GError **error) +static gboolean +load_file (const GValue *item, GValue *ret, gpointer data) { lua_State *L = data; + const gchar *path = g_value_get_string (item); + g_autoptr (GError) error = NULL; if (g_file_test (path, G_FILE_TEST_IS_DIR)) - return 0; + return TRUE; wp_info ("loading config file: %s", path); - if (!wplua_load_path (L, path, 0, 0, error)) - return -EINVAL; - return 0; + if (!wplua_load_path (L, path, 0, 0, &error)) { + g_value_unset (ret); + g_value_init (ret, G_TYPE_ERROR); + g_value_take_boxed (ret, g_steal_pointer (&error)); + return FALSE; + } + + g_value_set_int (ret, g_value_get_int (ret) + 1); + return TRUE; } gboolean wp_lua_scripting_load_configuration (const gchar * conf_file, WpCore * core, GError ** error) { - g_autofree gchar * path = NULL; g_autoptr (lua_State) L = wplua_new (); + g_autofree gchar * path = NULL; + g_autoptr (WpIterator) it = NULL; + g_auto (GValue) fold_ret = G_VALUE_INIT; gint nfiles = 0; wplua_enable_sandbox (L, WP_LUA_SANDBOX_MINIMAL_STD); @@ -113,7 +122,16 @@ wp_lua_scripting_load_configuration (const gchar * conf_file, g_clear_pointer (&path, g_free); path = g_strdup_printf ("%s.d", conf_file); - nfiles += wp_iter_config_files (path, ".lua", load_file, L, error); + it = wp_new_config_files_iterator (path, ".lua"); + + g_value_init (&fold_ret, G_TYPE_INT); + g_value_set_int (&fold_ret, nfiles); + if (!wp_iterator_fold (it, load_file, &fold_ret, L)) { + if (error && G_VALUE_HOLDS (&fold_ret, G_TYPE_ERROR)) + *error = g_value_dup_boxed (&fold_ret); + return FALSE; + } + nfiles = g_value_get_int (&fold_ret); if (nfiles == 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,