Fixes a segfault when H5Pset_mdc_log_options is called multiple times on a fapl (#601)

* Committing clang-format changes

* Fixes a segfault when H5Pset_mdc_log_options() is called multiple times

An internal string is incorrectly freed when the API call is invoked
multiple times on a property list, which will usually cause a segfault
to occur. On the first call the log location is NULL so the problem
doesn't occur.

Fixes HDFFV-11239

* Fixes typos

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

release_docs/RELEASE.txt

@@ -427,6 +427,18 @@ New Features
 
         (DER - 2021/04/28, HDFFV-11240)
 
+    - Fixes a segfault when H5Pset_mdc_log_options() is called multiple times
+
+      The call incorrectly attempts to free an internal copy of the previous
+      log location string, which causes a segfault. This only happens
+      when the call is invoked multiple times on the same property list.
+      On the first call to a given fapl, the log location is set to NULL so
+      the segfault does not occur.
+
+      The string is now handled properly and the segfault no longer occurs.
+
+        (DER - 2021/04/27, HDFFV-11239)
+      
     - HSYS_GOTO_ERROR now emits the results of GetLastError() on Windows
 
       HSYS_GOTO_ERROR is an internal macro that is used to produce error

src/H5Pfapl.c

@@ -4221,7 +4221,7 @@ herr_t
 H5Pset_mdc_log_options(hid_t plist_id, hbool_t is_enabled, const char *location, hbool_t start_on_access)
 {
     H5P_genplist_t *plist;               /* Property list pointer */
-    char *          tmp_location;        /* Working location pointer */
+    char *          new_location;        /* Working location pointer */
     herr_t          ret_value = SUCCEED; /* Return value */
 
     FUNC_ENTER_API(FAIL)
@@ -4237,19 +4237,14 @@ H5Pset_mdc_log_options(hid_t plist_id, hbool_t is_enabled, const char *location,
     if (NULL == (plist = H5P_object_verify(plist_id, H5P_FILE_ACCESS)))
         HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "plist_id is not a file access property list")
 
-    /* Get the current location string and free it */
-    if (H5P_get(plist, H5F_ACS_MDC_LOG_LOCATION_NAME, &tmp_location) < 0)
-        HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get current log location")
-    H5MM_xfree(tmp_location);
-
     /* Make a copy of the passed-in location */
-    if (NULL == (tmp_location = H5MM_xstrdup(location)))
+    if (NULL == (new_location = H5MM_xstrdup(location)))
         HGOTO_ERROR(H5E_PLIST, H5E_CANTCOPY, FAIL, "can't copy passed-in log location")
 
     /* Set values */
     if (H5P_set(plist, H5F_ACS_USE_MDC_LOGGING_NAME, &is_enabled) < 0)
         HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set is_enabled flag")
-    if (H5P_set(plist, H5F_ACS_MDC_LOG_LOCATION_NAME, &tmp_location) < 0)
+    if (H5P_set(plist, H5F_ACS_MDC_LOG_LOCATION_NAME, &new_location) < 0)
         HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set log location")
     if (H5P_set(plist, H5F_ACS_START_MDC_LOG_ON_ACCESS_NAME, &start_on_access) < 0)
         HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set start_on_access flag")

test/cache_logging.c

@@ -59,6 +59,12 @@ test_logging_api(void)
     if (H5Pset_mdc_log_options(fapl, is_enabled, LOG_LOCATION, start_on_access) < 0)
         TEST_ERROR;
 
+    /* Ensure that setting the property twice doesn't cause problems
+     * (addresses a previous bug).
+     */
+    if (H5Pset_mdc_log_options(fapl, is_enabled, LOG_LOCATION, start_on_access) < 0)
+        TEST_ERROR;
+
     /* Check to make sure that the property list getter returns the correct
      * location string buffer size;
      */