Fix loading plugin fails with missing directory GH issue HDFGroup#3248

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:

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 @@ H5PL_load(H5PL_type_t type, const H5PL_key_t *key)
  *
  * 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
  *

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))) {

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:

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,17 +92,17 @@ 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
  * If the value of \p plugin_control_mask is 0 (zero), all dynamic plugins
  * 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:

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;