Fix loading plugin fails with missing directory GH issue #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;