From 89b4afd4b317c5131cbc8607c99d21b369d050eb Mon Sep 17 00:00:00 2001 From: Allen Byrne <50328838+byrnHDF@users.noreply.github.com> Date: Wed, 2 Aug 2023 11:28:08 -0500 Subject: Fix loading plugin fails with missing directory GH issue #3248 (#3324) --- release_docs/RELEASE.txt | 7 ++++++- src/H5PLint.c | 36 ++++++++++++++++++++++++++++++++---- src/H5PLpath.c | 12 ++++++------ src/H5PLplugin_cache.c | 6 ++---- src/H5PLpublic.h | 9 ++++----- test/filter_plugin.c | 28 ++++++++++++++++++---------- 6 files changed, 68 insertions(+), 30 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 48a8e60..d6f3a57 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -124,7 +124,12 @@ New Features Library: -------- - - + - Change the error handling for a not found path in the find plugin process. + + While attempting to load a plugin the HDF5 library will fail if one of the + directories in the plugin paths does not exist, even if there are more paths + to check. Instead of exiting the function with an error, just logged the error + and continue processing the list of paths to check. Parallel Library: diff --git a/src/H5PLint.c b/src/H5PLint.c index e29bc10..1b58be1 100644 --- a/src/H5PLint.c +++ b/src/H5PLint.c @@ -258,11 +258,17 @@ H5PL_load(H5PL_type_t type, const H5PL_key_t *key) /* If not found, try iterating through the path table to find an appropriate plugin */ if (!found) if (H5PL__find_plugin_in_path_table(&search_params, &found, &plugin_info) < 0) - HGOTO_ERROR(H5E_PLUGIN, H5E_CANTGET, NULL, "search in path table failed") + HGOTO_ERROR(H5E_PLUGIN, H5E_CANTGET, NULL, + "can't find plugin in the paths either set by HDF5_PLUGIN_PATH, or default location, " + "or set by H5PLxxx functions") /* Set the return value we found the plugin */ if (found) ret_value = plugin_info; + else + HGOTO_ERROR(H5E_PLUGIN, H5E_NOTFOUND, NULL, + "can't find plugin. Check either HDF5_PLUGIN_PATH, default location, " + "or path set by H5PLxxx functions") done: FUNC_LEAVE_NOAPI(ret_value) @@ -273,9 +279,31 @@ done: * * Purpose: Opens a plugin. * - * The success parameter will be set to TRUE and the plugin_info - * parameter will be filled in on success. Otherwise, they - * will be FALSE and NULL, respectively. + * `path` specifies the path to the plugin library file. + * + * `type` specifies the type of plugin being searched for and + * will be used to verify that a loaded plugin matches the + * type requested. H5PL_TYPE_NONE may be passed, in which case + * no plugin type verification is performed. This is most + * useful when iterating over available plugins without regard + * to their types. + * + * `key` specifies the information that will be used to find a + * specific plugin. For filter plugins, this is typically an + * integer identifier. For VOL connectors, this + * is typically either an integer identifier or a name string. + * After a plugin has been opened, this information will be + * compared against the relevant information provided by the + * plugin to ensure that the plugin is a match. If + * H5PL_TYPE_NONE is provided for `type`, then `key` should be + * NULL. + * + * On successful open of a plugin, the `success` parameter + * will be set to TRUE and the `plugin_type` and `plugin_info` + * parameters will be filled appropriately. On failure, the + * `success` parameter will be set to FALSE, the `plugin_type` + * parameter will be set to H5PL_TYPE_ERROR and the + * `plugin_info` parameter will be set to NULL. * * Return: SUCCEED/FAIL * diff --git a/src/H5PLpath.c b/src/H5PLpath.c index a6734cb..10a668c 100644 --- a/src/H5PLpath.c +++ b/src/H5PLpath.c @@ -615,9 +615,9 @@ H5PL__path_table_iterate_process_path(const char *plugin_path, H5PL_iterate_type HDassert(plugin_path); HDassert(iter_op); - /* Open the directory */ + /* Open the directory - skip the path if the directory can't be opened */ if (!(dirp = HDopendir(plugin_path))) - HGOTO_ERROR(H5E_PLUGIN, H5E_OPENERROR, H5_ITER_ERROR, "can't open directory: %s", plugin_path) + HGOTO_DONE(H5_ITER_CONT); /* Iterate through all entries in the directory */ while (NULL != (dp = HDreaddir(dirp))) { @@ -710,7 +710,7 @@ H5PL__path_table_iterate_process_path(const char *plugin_path, H5PL_iterate_type * skip the path if the directory can't be opened */ HDsnprintf(service, sizeof(service), "%s\\*.dll", plugin_path); if ((hFind = FindFirstFileA(service, &fdFile)) == INVALID_HANDLE_VALUE) - HGOTO_ERROR(H5E_PLUGIN, H5E_OPENERROR, H5_ITER_ERROR, "can't open directory") + HGOTO_DONE(H5_ITER_CONT); /* Loop over all the files */ do { @@ -799,8 +799,7 @@ H5PL__find_plugin_in_path_table(const H5PL_search_params_t *search_params, hbool /* Search for the plugin in this path */ if (H5PL__find_plugin_in_path(search_params, found, H5PL_paths_g[u], plugin_info) < 0) - HGOTO_ERROR(H5E_PLUGIN, H5E_CANTGET, FAIL, "search in path %s encountered an error", - H5PL_paths_g[u]) + HERROR(H5E_PLUGIN, H5E_CANTGET, "search in path %s encountered an error", H5PL_paths_g[u]); /* Break out if found */ if (*found) { @@ -852,7 +851,8 @@ H5PL__find_plugin_in_path(const H5PL_search_params_t *search_params, hbool_t *fo /* Open the directory */ if (!(dirp = HDopendir(dir))) - HGOTO_ERROR(H5E_PLUGIN, H5E_OPENERROR, FAIL, "can't open directory: %s", dir) + HGOTO_ERROR(H5E_PLUGIN, H5E_OPENERROR, FAIL, "can't open directory (%s). Please verify its existence", + dir) /* Iterate through all entries in the directory */ while (NULL != (dp = HDreaddir(dirp))) { diff --git a/src/H5PLplugin_cache.c b/src/H5PLplugin_cache.c index cf0fd23..b74f6a1 100644 --- a/src/H5PLplugin_cache.c +++ b/src/H5PLplugin_cache.c @@ -273,7 +273,7 @@ H5PL__find_plugin_in_cache(const H5PL_search_params_t *search_params, hbool_t *f /* Get the "get plugin info" function from the plugin. */ if (NULL == (get_plugin_info_function = (H5PL_get_plugin_info_t)H5PL_GET_LIB_FUNC( - (H5PL_cache_g[u]).handle, "H5PLget_plugin_info"))) + H5PL_cache_g[u].handle, "H5PLget_plugin_info"))) HGOTO_ERROR(H5E_PLUGIN, H5E_CANTGET, FAIL, "can't get function for H5PLget_plugin_info") /* Call the "get plugin info" function */ @@ -286,9 +286,7 @@ H5PL__find_plugin_in_cache(const H5PL_search_params_t *search_params, hbool_t *f /* No need to continue processing */ break; - - } /* end if */ - + } } /* end for */ done: diff --git a/src/H5PLpublic.h b/src/H5PLpublic.h index a886375..8ef1c3e 100644 --- a/src/H5PLpublic.h +++ b/src/H5PLpublic.h @@ -17,8 +17,7 @@ #ifndef H5PLpublic_H #define H5PLpublic_H -/* Public headers needed by this file */ -#include "H5public.h" /* Generic Functions */ +#include "H5public.h" /* Generic Functions */ /*******************/ /* Public Typedefs */ @@ -93,9 +92,9 @@ H5_DLL herr_t H5PLset_loading_state(unsigned int plugin_control_mask); * \brief Queries the loadability of dynamic plugin types * * \param[out] plugin_control_mask List of dynamic plugin types that are enabled or disabled.\n - * A plugin bit set to 0 (zero) indicates that that the dynamic plugin type is + * A plugin bit set to 0 (zero) indicates that the dynamic plugin type is * disabled.\n - * A plugin bit set to 1 (one) indicates that that the dynamic plugin type is + * A plugin bit set to 1 (one) indicates that the dynamic plugin type is * enabled.\n * If the value of \p plugin_control_mask is negative, all dynamic plugin * types are enabled.\n @@ -103,7 +102,7 @@ H5_DLL herr_t H5PLset_loading_state(unsigned int plugin_control_mask); * are disabled. * \return \herr_t * - * \details H5PLget_loading_state() retrieves the bitmask that controls whether a certain type of plugins + * \details H5PLget_loading_state() retrieves the bitmask that controls whether a certain type of plugin * (e.g.: filters, VOL drivers) will be loaded by the HDF5 library. * * Bit positions allocated to date are specified in \ref H5PL_type_t as follows: diff --git a/test/filter_plugin.c b/test/filter_plugin.c index ea5c000..7ed8ced 100644 --- a/test/filter_plugin.c +++ b/test/filter_plugin.c @@ -438,9 +438,11 @@ static herr_t test_dataset_write_with_filters(hid_t fid) { hid_t dcpl_id = -1; /* Dataset creation property list ID */ - unsigned int compress_level; /* Deflate compression level */ unsigned int filter1_data; /* Data used by filter 1 */ unsigned int libver_values[4]; /* Used w/ the filter that makes HDF5 calls */ +#ifdef H5_HAVE_FILTER_DEFLATE + unsigned int compress_level; /* Deflate compression level */ +#endif /*---------------------------------------------------------- * STEP 1: Test deflation by itself. @@ -847,7 +849,7 @@ test_creating_groups_using_plugins(hid_t fid) for (i = 0; i < N_SUBGROUPS; i++) { char *sp = subgroup_name; - sp += HDsprintf(subgroup_name, SUBGROUP_PREFIX); + sp += HDsnprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX); HDsprintf(sp, "%d", i); if ((sub_gid = H5Gcreate2(gid, subgroup_name, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) @@ -906,7 +908,7 @@ test_opening_groups_using_plugins(hid_t fid) for (i = 0; i < N_SUBGROUPS; i++) { char *sp = subgroup_name; - sp += HDsprintf(subgroup_name, SUBGROUP_PREFIX); + sp += HDsnprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX); HDsprintf(sp, "%d", i); if ((sub_gid = H5Gopen2(gid, subgroup_name, H5P_DEFAULT)) < 0) @@ -1025,7 +1027,7 @@ test_path_api_calls(void) /* Add a bunch of paths to the path table */ for (u = 0; u < n_starting_paths; u++) { - HDsprintf(path, "a_path_%u", u); + HDsnprintf(path, sizeof(path), "a_path_%u", u); if (H5PLappend(path) < 0) { HDfprintf(stderr, " at %u: %s\n", u, path); TEST_ERROR; @@ -1091,7 +1093,7 @@ test_path_api_calls(void) /* Get path at the last index */ if ((path_len = H5PLget(n_starting_paths - 1, path, 256)) <= 0) TEST_ERROR; - HDsprintf(temp_name, "a_path_%u", n_starting_paths - 1); + HDsnprintf(temp_name, sizeof(temp_name), "a_path_%u", n_starting_paths - 1); if (HDstrcmp(path, temp_name) != 0) { HDfprintf(stderr, " get %u: %s\n", n_starting_paths - 1, path); TEST_ERROR; @@ -1145,7 +1147,7 @@ test_path_api_calls(void) TESTING(" prepend"); /* Prepend one path */ - HDsprintf(path, "a_path_%d", n_starting_paths + 1); + HDsnprintf(path, sizeof(path), "a_path_%d", n_starting_paths + 1); if (H5PLprepend(path) < 0) { HDfprintf(stderr, " prepend %u: %s\n", n_starting_paths + 1, path); TEST_ERROR; @@ -1168,7 +1170,7 @@ test_path_api_calls(void) /* Verify that the path was inserted at index zero */ if (H5PLget(0, path, 256) <= 0) TEST_ERROR; - HDsprintf(temp_name, "a_path_%d", n_starting_paths + 1); + HDsnprintf(temp_name, sizeof(temp_name), "a_path_%d", n_starting_paths + 1); if (HDstrcmp(path, temp_name) != 0) { HDfprintf(stderr, " get 0: %s\n", path); TEST_ERROR; @@ -1183,7 +1185,7 @@ test_path_api_calls(void) TESTING(" replace"); /* Replace one path at index 1 */ - HDsprintf(path, "a_path_%u", n_starting_paths + 4); + HDsnprintf(path, sizeof(path), "a_path_%u", n_starting_paths + 4); if (H5PLreplace(path, 1) < 0) { HDfprintf(stderr, " replace 1: %s\n", path); TEST_ERROR; @@ -1202,7 +1204,7 @@ test_path_api_calls(void) /* Check path at index 0 */ if (H5PLget(0, path, 256) <= 0) TEST_ERROR; - HDsprintf(temp_name, "a_path_%u", n_starting_paths + 1); + HDsnprintf(temp_name, sizeof(temp_name), "a_path_%u", n_starting_paths + 1); if (HDstrcmp(path, temp_name) != 0) { HDfprintf(stderr, " get 0: %s\n", path); TEST_ERROR; @@ -1250,7 +1252,7 @@ test_path_api_calls(void) TESTING(" insert"); /* Insert one path at index 3*/ - HDsprintf(path, "a_path_%d", n_starting_paths + 5); + HDsnprintf(path, sizeof(path), "a_path_%d", n_starting_paths + 5); if (H5PLinsert(path, 3) < 0) { HDfprintf(stderr, " insert 3: %s\n", path); TEST_ERROR; @@ -1436,6 +1438,12 @@ main(void) else my_fapl_id = old_ff_fapl_id; + /* Add extra path to check for correct error process */ + if (H5PLprepend("bogus") < 0) { + fprintf(stderr, "Could not prepend path:bogus\n"); + TEST_ERROR; + } + /* Reopen the file for testing data reading */ if ((fid = H5Fopen(filename, H5F_ACC_RDONLY, my_fapl_id)) < 0) TEST_ERROR; -- cgit v0.12