Enable the Onion VFD to support H5Pset_driver_by_name (#1813)

* Enable the Onion VFD to support H5Pset_driver_by_name and added a test case for it.

* Committing clang-format changes

* Revised a private function to return an error if it fails.

* Committing clang-format changes

* Refactored some code in the previous commit.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

2 files changed, 403 insertions, 12 deletions

diff --git a/src/H5FDonion.c b/src/H5FDonion.c
index c51ed23..6d42a26 100644
--- a/src/H5FDonion.c
+++ b/src/H5FDonion.c
@@ -772,6 +772,126 @@ done:
     FUNC_LEAVE_NOAPI(ret_value);
 } /* end H5FD__onion_create_truncate_onion() */
 
+static herr_t
+H5FD__onion_remove_unused_symbols(char *s)
+{
+    char *d = s;
+
+    FUNC_ENTER_PACKAGE_NOERR;
+
+    do {
+        while (*d == '{' || *d == '}' || *d == ' ') {
+            ++d;
+        }
+    } while ((*s++ = *d++));
+
+    FUNC_LEAVE_NOAPI(SUCCEED);
+}
+
+static herr_t
+H5FD__onion_parse_config_str(char *config_str, H5FD_onion_fapl_info_t **info)
+{
+    H5FD_onion_fapl_info_t *fa        = NULL;
+    herr_t                  ret_value = SUCCEED;
+
+    FUNC_ENTER_PACKAGE;
+
+    if (!HDstrcmp(config_str, ""))
+        HGOTO_ERROR(H5E_PLIST, H5E_BADVALUE, FAIL, "configure string can't be empty")
+
+    fa = (H5FD_onion_fapl_info_t *)H5MM_calloc(sizeof(H5FD_onion_fapl_info_t));
+
+    /* Initialize to the default values */
+    fa->version          = H5FD_ONION_FAPL_INFO_VERSION_CURR;
+    fa->backing_fapl_id  = H5P_DEFAULT;
+    fa->page_size        = 4;
+    fa->store_target     = H5FD_ONION_STORE_TARGET_ONION;
+    fa->revision_num     = H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST;
+    fa->force_write_open = 0;
+    fa->creation_flags   = 0;
+    HDstrcpy(fa->comment, "initial comment");
+
+    /* If a single integer is passed in as a string, it's a shortcut for the tools
+     * (h5repack, h5diff, h5dump).  Otherwise, the string should have curly brackets,
+     * e.g. {revision_num: 2; page_size: 4;}
+     */
+    if (config_str[0] != '{')
+        fa->revision_num = (uint64_t)HDstrtoull(config_str, NULL, 10);
+    else {
+        char *token1 = NULL, *token2 = NULL;
+
+        /* Remove the curly brackets and space from the configure string */
+        H5FD__onion_remove_unused_symbols(config_str);
+
+        /* The configure string can't be empty after removing the curly brackets */
+        if (!HDstrcmp(config_str, ""))
+            HGOTO_ERROR(H5E_PLIST, H5E_BADVALUE, FAIL, "configure string can't be empty")
+
+        token1 = HDstrtok(config_str, ":");
+        token2 = HDstrtok(NULL, ";");
+
+        do {
+            if (token1 && token2) {
+                if (!HDstrcmp(token1, "version")) {
+                    if (!HDstrcmp(token2, "H5FD_ONION_FAPL_INFO_VERSION_CURR"))
+                        fa->version = H5FD_ONION_FAPL_INFO_VERSION_CURR;
+                }
+                else if (!HDstrcmp(token1, "backing_fapl_id")) {
+                    if (!HDstrcmp(token2, "H5P_DEFAULT"))
+                        fa->backing_fapl_id = H5P_DEFAULT;
+                    else if (!strcmp(token2, "H5I_INVALID_HID"))
+                        fa->backing_fapl_id = H5I_INVALID_HID;
+                    else
+                        fa->backing_fapl_id = HDstrtoll(token2, NULL, 10);
+                }
+                else if (!HDstrcmp(token1, "page_size")) {
+                    fa->page_size = (uint32_t)HDstrtoul(token2, NULL, 10);
+                }
+                else if (!HDstrcmp(token1, "revision_num")) {
+                    if (!HDstrcmp(token2, "H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST"))
+                        fa->revision_num = H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST;
+                    else
+                        fa->revision_num = (uint64_t)HDstrtoull(token2, NULL, 10);
+                }
+                else if (!HDstrcmp(token1, "force_write_open")) {
+                    fa->force_write_open = (uint8_t)HDstrtoul(token2, NULL, 10);
+                }
+                else if (!HDstrcmp(token1, "creation_flags")) {
+                    fa->creation_flags = (uint8_t)HDstrtoul(token2, NULL, 10);
+                }
+                else if (!HDstrcmp(token1, "comment")) {
+                    HDstrcpy(fa->comment, token2);
+                }
+                else
+                    HGOTO_ERROR(H5E_PLIST, H5E_BADVALUE, FAIL, "unknown token in the configure string: %s",
+                                token1)
+            }
+
+            token1 = HDstrtok(NULL, ":");
+            token2 = HDstrtok(NULL, ";");
+        } while (token1);
+    }
+
+    if (H5P_DEFAULT == fa->backing_fapl_id || H5I_INVALID_HID == fa->backing_fapl_id) {
+        H5P_genclass_t *pclass; /* Property list class to modify */
+
+        if (NULL == (pclass = (H5P_genclass_t *)H5I_object_verify(H5P_FILE_ACCESS, H5I_GENPROP_CLS)))
+            HGOTO_ERROR(H5E_PLIST, H5E_BADTYPE, FAIL, "not a property list class");
+
+        /* Create the new property list */
+        if ((fa->backing_fapl_id = H5P_create_id(pclass, TRUE)) < 0)
+            HGOTO_ERROR(H5E_PLIST, H5E_CANTCREATE, FAIL, "unable to create property list");
+    }
+
+    *info = fa;
+
+done:
+    if (FAIL == ret_value)
+        H5MM_free(fa);
+
+    FUNC_LEAVE_NOAPI(ret_value);
+}
+
 /*-----------------------------------------------------------------------------
  * Function:    H5FD__onion_open
  *
@@ -784,16 +904,16 @@ done:
 static H5FD_t *
 H5FD__onion_open(const char *filename, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
 {
-    H5P_genplist_t *              plist = NULL;
-    H5FD_onion_t *                file  = NULL;
-    const H5FD_onion_fapl_info_t *fa    = NULL;
-    ;
-    hid_t   backing_fapl_id       = H5I_INVALID_HID;
-    char *  name_onion            = NULL;
-    char *  recovery_file_nameery = NULL;
-    H5FD_t *ret_value             = NULL;
-    bool    new_open              = false;
-    haddr_t canon_eof             = 0;
+    H5P_genplist_t *        plist                 = NULL;
+    H5FD_onion_t *          file                  = NULL;
+    H5FD_onion_fapl_info_t *fa                    = NULL;
+    char *                  config_str            = NULL;
+    hid_t                   backing_fapl_id       = H5I_INVALID_HID;
+    char *                  name_onion            = NULL;
+    char *                  recovery_file_nameery = NULL;
+    H5FD_t *                ret_value             = NULL;
+    bool                    new_open              = false;
+    haddr_t                 canon_eof             = 0;
 
     FUNC_ENTER_PACKAGE
 
@@ -805,8 +925,17 @@ H5FD__onion_open(const char *filename, unsigned flags, hid_t fapl_id, haddr_t ma
     HDassert(H5P_DEFAULT != fapl_id);
     if (NULL == (plist = (H5P_genplist_t *)H5I_object(fapl_id)))
         HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, NULL, "not a file access property list")
-    if (NULL == (fa = (const H5FD_onion_fapl_info_t *)H5P_peek_driver_info(plist)))
-        HGOTO_ERROR(H5E_PLIST, H5E_BADVALUE, NULL, "bad VFL driver info")
+
+    fa = (H5FD_onion_fapl_info_t *)H5P_peek_driver_info(plist);
+
+    /* This VFD can be invoked by either H5Pset_fapl_onion or H5Pset_driver_by_name */
+    if (NULL == fa) {
+        if (NULL == (config_str = (char *)H5P_peek_driver_config_str(plist)))
+            HGOTO_ERROR(H5E_PLIST, H5E_BADVALUE, NULL, "bad VFL driver configure string")
+
+        if (H5FD__onion_parse_config_str(config_str, &fa) < 0)
+            HGOTO_ERROR(H5E_PLIST, H5E_BADVALUE, NULL, "failed to parse configure string")
+    }
 
     /* Check for unsupported target values */
     if (H5FD_ONION_STORE_TARGET_H5 == fa->store_target)
@@ -931,6 +1060,7 @@ H5FD__onion_open(const char *filename, unsigned flags, hid_t fapl_id, haddr_t ma
                 file->logical_eof = canon_eof;
 
                 backing_fapl_id = H5FD__onion_get_legit_fapl_id(file->fa.backing_fapl_id);
+
                 if (H5I_INVALID_HID == backing_fapl_id)
                     HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "invalid backing FAPL ID")
 
@@ -1065,6 +1195,10 @@ done:
     H5MM_xfree(name_onion);
     H5MM_xfree(recovery_file_nameery);
 
+    if (config_str && fa)
+        if (H5I_GENPROP_LST == H5I_get_type(fa->backing_fapl_id))
+            H5I_dec_app_ref(fa->backing_fapl_id);
+
     if ((NULL == ret_value) && file) {
 
         if (file->original_file)
diff --git a/test/onion.c b/test/onion.c
index 6633918..ed0ecc6 100644
--- a/test/onion.c
+++ b/test/onion.c
@@ -4602,6 +4602,262 @@ error:
     return -1;
 }
 
+static int
+test_integration_create_by_name(void)
+{
+    const char *            basename   = "integration_by_name.h5";
+    hid_t                   fapl_id    = H5I_INVALID_HID;
+    struct onion_filepaths *paths      = NULL;
+    H5FD_onion_fapl_info_t  onion_info = {
+        H5FD_ONION_FAPL_INFO_VERSION_CURR,
+        H5P_DEFAULT,                   /* backing_fapl_id  */
+        ONION_TEST_PAGE_SIZE_1,        /* page_size        */
+        H5FD_ONION_STORE_TARGET_ONION, /* store_target     */
+        H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST,
+        0,               /* force_write_open */
+        0,               /* creation flags, was H5FD_ONION_FAPL_INFO_CREATE_FLAG_ENABLE_PAGE_ALIGNMENT */
+        "initial commit" /* comment          */
+    };
+    hid_t   file_id    = H5I_INVALID_HID;
+    hid_t   file       = H5I_INVALID_HID;
+    hid_t   space      = H5I_INVALID_HID;
+    hid_t   dset       = H5I_INVALID_HID;
+    hid_t   dcpl       = H5I_INVALID_HID;
+    hsize_t dims[2]    = {1, ONE_DIM_SIZE};
+    hsize_t maxdims[2] = {1, ONE_DIM_SIZE};
+    int     fillval;
+    struct {
+        int arr[ONE_DIM_SIZE];
+    } *wdata = NULL;
+    struct {
+        int arr[ONE_DIM_SIZE];
+    } *rdata = NULL;
+    struct {
+        int arr[ONE_DIM_SIZE];
+    } *dset_data = NULL;
+
+    TESTING("H5Pset_driver_by_name");
+
+    /* Setup */
+    if (NULL == (wdata = HDcalloc(1, sizeof(*wdata))))
+        TEST_ERROR;
+    if (NULL == (rdata = HDcalloc(1, sizeof(*rdata))))
+        TEST_ERROR;
+    if (NULL == (dset_data = HDcalloc(1, sizeof(*dset_data))))
+        TEST_ERROR;
+
+    onion_info.backing_fapl_id = h5_fileaccess();
+    if ((fapl_id = H5Pcreate(H5P_FILE_ACCESS)) < 0)
+        TEST_ERROR;
+
+    /* Use H5Pset_driver_by_name to enable the Onion VFD */
+    if (H5Pset_driver_by_name(fapl_id, "onion", "{revision_num: H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST}") <
+        0)
+        TEST_ERROR;
+
+    if (NULL == (paths = onion_filepaths_init(basename, &onion_info)))
+        TEST_ERROR;
+
+    HDremove(paths->canon);
+    HDremove(paths->onion);
+    HDremove(paths->recovery);
+
+    /*----------------------------------------------------------------------
+     * Create the skeleton file (create the file without Onion VFD)
+     *----------------------------------------------------------------------
+     */
+    /* Initialize data */
+    for (int i = 0; i < ONE_DIM_SIZE; i++)
+        wdata->arr[i] = i;
+
+    /* Create a new file using the default properties */
+    if ((file = H5Fcreate(paths->canon, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+
+    /* Create dataspace with unlimited dimensions*/
+    if ((space = H5Screate_simple(2, dims, maxdims)) < 0)
+        TEST_ERROR;
+
+    /* Create the dataset creation property list */
+    if ((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0)
+        TEST_ERROR;
+
+    /* Set the fill value for the dataset */
+    fillval = 99;
+    if (H5Pset_fill_value(dcpl, H5T_NATIVE_INT, &fillval) < 0)
+        TEST_ERROR;
+
+    /* Set the allocation time to "early".  This way we can be sure
+     * that reading from the dataset immediately after creation will
+     * return the fill value.
+     */
+    if (H5Pset_alloc_time(dcpl, H5D_ALLOC_TIME_EARLY) < 0)
+        TEST_ERROR;
+
+    /* Create the dataset using the dataset creation property list */
+    if ((dset = H5Dcreate(file, "DS1", H5T_STD_I32LE, space, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+
+    /* Write the data to the dataset */
+    if (H5Dwrite(dset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, wdata) < 0)
+        TEST_ERROR;
+
+    /* Close everything */
+    if (H5Pclose(dcpl) < 0)
+        TEST_ERROR;
+    if (H5Dclose(dset) < 0)
+        TEST_ERROR;
+    if (H5Sclose(space) < 0)
+        TEST_ERROR;
+    if (H5Fclose(file) < 0)
+        TEST_ERROR;
+
+    /*----------------------------------------------------------------------
+     * First revision: open the file with Onion VFD and change the data
+     *----------------------------------------------------------------------
+     */
+    if ((file_id = H5Fopen(paths->canon, H5F_ACC_RDWR, fapl_id)) < 0)
+        TEST_ERROR;
+
+    if ((dset = H5Dopen(file_id, "DS1", H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+
+    for (int i = 0; i < ONE_DIM_SIZE; i++)
+        dset_data->arr[i] = i + ONE_DIM_SIZE;
+
+    if (H5Dwrite(dset, H5T_STD_I32LE, H5S_ALL, H5S_ALL, H5P_DEFAULT, dset_data) < 0)
+        TEST_ERROR;
+
+    if (H5Dclose(dset) < 0)
+        TEST_ERROR;
+    dset = H5I_INVALID_HID;
+    if (H5Fclose(file_id) < 0)
+        TEST_ERROR;
+    file_id = H5I_INVALID_HID;
+
+    /*----------------------------------------------------------------------
+     * Second revision: open the file with Onion VFD and change the data
+     *----------------------------------------------------------------------
+     */
+    if ((file_id = H5Fopen(paths->canon, H5F_ACC_RDWR, fapl_id)) < 0)
+        TEST_ERROR;
+    if ((dset = H5Dopen(file_id, "DS1", H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+
+    for (int i = 0; i < ONE_DIM_SIZE; i++)
+        dset_data->arr[i] = i + 2048;
+
+    if (H5Dwrite(dset, H5T_STD_I32LE, H5S_ALL, H5S_ALL, H5P_DEFAULT, dset_data) < 0)
+        TEST_ERROR;
+
+    /* CLEANUP */
+    if (H5Dclose(dset) < 0)
+        TEST_ERROR;
+    dset = H5I_INVALID_HID;
+    if (H5Fclose(file_id) < 0)
+        TEST_ERROR;
+    file_id = H5I_INVALID_HID;
+
+    /*----------------------------------------------------------------------
+     * Third revision: open the file with Onion VFD and change the data
+     *----------------------------------------------------------------------
+     */
+    if ((file_id = H5Fopen(paths->canon, H5F_ACC_RDWR, fapl_id)) < 0)
+        TEST_ERROR;
+    if ((dset = H5Dopen(file_id, "DS1", H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+
+    for (int i = 0; i < ONE_DIM_SIZE; i += 20)
+        dset_data->arr[i] = i + 3072;
+
+    if (H5Dwrite(dset, H5T_STD_I32LE, H5S_ALL, H5S_ALL, H5P_DEFAULT, dset_data) < 0)
+        TEST_ERROR;
+
+    /* CLEANUP */
+    if (H5Dclose(dset) < 0)
+        TEST_ERROR;
+    dset = H5I_INVALID_HID;
+    if (H5Fclose(file_id) < 0)
+        TEST_ERROR;
+    file_id = H5I_INVALID_HID;
+    if (H5Pclose(fapl_id) < 0)
+        TEST_ERROR;
+    fapl_id = H5I_INVALID_HID;
+
+    /*----------------------------------------------------------------------
+     *  Start to verify the revision with H5Pset_driver_by_name
+     *----------------------------------------------------------------------
+     */
+    /*----------------------------------------------------------------------
+     * Verify the second revision
+     *----------------------------------------------------------------------
+     */
+    if ((fapl_id = H5Pcreate(H5P_FILE_ACCESS)) < 0)
+        TEST_ERROR;
+    if (H5Pset_driver_by_name(fapl_id, "onion", "{revision_num: 2; page_size: 4; }") < 0)
+        TEST_ERROR;
+
+    if ((file_id = H5Fopen(paths->canon, H5F_ACC_RDONLY, fapl_id)) < 0)
+        TEST_ERROR;
+    if ((dset = H5Dopen(file_id, "DS1", H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+
+    if (H5Dread(dset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, rdata) < 0)
+        TEST_ERROR;
+
+    for (int i = 0; i < ONE_DIM_SIZE; i += 20) {
+        int expected = i + 2048;
+        if (rdata->arr[i] != expected) {
+            HDprintf("ERROR!!! Expected: %d, Got: %d\n", expected, rdata->arr[i]);
+            TEST_ERROR;
+        }
+    }
+
+    /* Close everything */
+    if (H5Dclose(dset) < 0)
+        TEST_ERROR;
+    if (H5Fclose(file_id) < 0)
+        TEST_ERROR;
+
+    if (H5Pclose(fapl_id) < 0)
+        TEST_ERROR;
+
+    HDremove(paths->canon);
+    HDremove(paths->onion);
+    HDremove(paths->recovery);
+    onion_filepaths_destroy(paths);
+
+    HDfree(wdata);
+    HDfree(rdata);
+    HDfree(dset_data);
+
+    PASSED();
+    return 0;
+
+error:
+
+    if (paths != NULL) {
+        HDremove(paths->canon);
+        HDremove(paths->onion);
+        HDremove(paths->recovery);
+        onion_filepaths_destroy(paths);
+    }
+
+    H5E_BEGIN_TRY
+    {
+        H5Dclose(dset);
+        H5Fclose(file_id);
+        H5Pclose(fapl_id);
+    }
+    H5E_END_TRY;
+
+    HDfree(wdata);
+    HDfree(rdata);
+    HDfree(dset_data);
+
+    return -1;
+} /* end test_integration_create_simple() */
+
 /*-----------------------------------------------------------------------------
  *
  * Function:     main()
@@ -4645,6 +4901,7 @@ main(void)
     nerrors -= test_integration_dset_extension();
     nerrors -= test_integration_ctl();
     nerrors -= test_integration_reference();
+    nerrors -= test_integration_create_by_name();
 
     if (nerrors > 0) {
         HDprintf("***** %d Onion TEST%s FAILED! *****\n", nerrors, nerrors > 1 ? "S" : "");