Add error checking to the minimized dset header size calculation.

Update printf->HDprintf statements.

2 files changed, 65 insertions, 31 deletions

diff --git a/src/H5Dint.c b/src/H5Dint.c
index 7c22835..7a39d4c 100644
--- a/src/H5Dint.c
+++ b/src/H5Dint.c
@@ -708,8 +708,8 @@ done:
  *
  * Purpose:    Calculate the size required for the minimized object header.
  *
- * Return:     Success: SUCCEED (0) (non-negative value)
- *             Failure: FAIL (-1) (negative value)
+ * Return:     Success: Positive value > 0
+ *             Failure: 0
  *
  * Programmer: Jacob Smith
  *             16 August 2018
@@ -722,9 +722,10 @@ H5D__calculate_minimum_header_size(H5F_t *file, H5D_t *dset, H5O_t *ohdr)
     H5O_fill_t *fill_prop        = NULL;
     hbool_t     use_at_least_v18 = FALSE;
     const char  continuation[1]  = ""; /* requred for work-around */
+    size_t      get_value        = 0;
     size_t      ret_value        = 0;
 
-    FUNC_ENTER_NOAPI_NOINIT_NOERR;
+    FUNC_ENTER_NOAPI_NOINIT;
 
     HDassert(file);
     HDassert(dset);
@@ -735,22 +736,37 @@ H5D__calculate_minimum_header_size(H5F_t *file, H5D_t *dset, H5O_t *ohdr)
     use_at_least_v18 = (H5F_LOW_BOUND(file) >= H5F_LIBVER_V18);
 
     /* Datatype message size */
-    ret_value += H5O_msg_size_oh(file, ohdr, H5O_DTYPE_ID, type, 0);
+    get_value = H5O_msg_size_oh(file, ohdr, H5O_DTYPE_ID, type, 0);
+    if (get_value == 0)
+        HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "Can't get size of datatype message")
+    ret_value += get_value;
 
     /* Shared Dataspace message size */
-    ret_value += H5O_msg_size_oh(file, ohdr, H5O_SDSPACE_ID, dset->shared->space, 0);
+    get_value = H5O_msg_size_oh(file, ohdr, H5O_SDSPACE_ID, dset->shared->space, 0);
+    if (get_value == 0)
+        HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of dataspace message")
+    ret_value += get_value;
 
     /* "Layout" message size */
-    ret_value += H5O_msg_size_oh(file, ohdr, H5O_LAYOUT_ID, &dset->shared->layout, 0);
+    get_value = H5O_msg_size_oh(file, ohdr, H5O_LAYOUT_ID, &dset->shared->layout, 0);
+    if (get_value == 0)
+        HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of layout message")
+    ret_value += get_value;
 
     /* Fill Value message size */
-    ret_value += H5O_msg_size_oh(file, ohdr, H5O_FILL_NEW_ID, fill_prop, 0);
+    get_value = H5O_msg_size_oh(file, ohdr, H5O_FILL_NEW_ID, fill_prop, 0);
+    if (get_value == 0)
+        HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of fill value message")
+    ret_value += get_value;
 
     /* "Continuation" message size */
     /* message pointer "continuation" is unused by raw get function, however,
      * a null pointer would be intercepted by an assert in H5O_msg_size_oh().
      */
-    ret_value += H5O_msg_size_oh(file, ohdr, H5O_CONT_ID, continuation, 0);
+    get_value = H5O_msg_size_oh(file, ohdr, H5O_CONT_ID, continuation, 0);
+    if (get_value == 0)
+        HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of continuation message")
+    ret_value += get_value;
 
     /* Fill Value (backwards compatability) message size */
     if(fill_prop->buf && !use_at_least_v18) {
@@ -760,21 +776,33 @@ H5D__calculate_minimum_header_size(H5F_t *file, H5D_t *dset, H5O_t *ohdr)
         /* guards against shared component modification */
         HDmemcpy(&old_fill_prop, fill_prop, sizeof(old_fill_prop));
 
-        H5O_msg_reset_share(H5O_FILL_ID, &old_fill_prop);
+        if (H5O_msg_reset_share(H5O_FILL_ID, &old_fill_prop) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't reset the copied fill property")
 
-        ret_value += H5O_msg_size_oh(file, ohdr, H5O_FILL_ID, &old_fill_prop, 0);
+        get_value = H5O_msg_size_oh(file, ohdr, H5O_FILL_ID, &old_fill_prop, 0);
+        if (get_value == 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of fill value (backwards compat) message")
+        ret_value += get_value;
     }
 
     /* Filter/Pipeline message size */
     if(H5D_CHUNKED == dset->shared->layout.type) {
         H5O_pline_t *pline = &dset->shared->dcpl_cache.pline;
-        if(pline->nused > 0)
-            ret_value += H5O_msg_size_oh(file, ohdr, H5O_PLINE_ID, pline, 0);
+        if(pline->nused > 0) {
+            get_value = H5O_msg_size_oh(file, ohdr, H5O_PLINE_ID, pline, 0);
+            if (get_value == 0)
+                HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of filter message")
+            ret_value += get_value;
+        }
     }
 
     /* External File Link message size */
-    if(dset->shared->dcpl_cache.efl.nused > 0)
-        ret_value += H5O_msg_size_oh(file, ohdr, H5O_EFL_ID, &dset->shared->dcpl_cache.efl, 0);
+    if(dset->shared->dcpl_cache.efl.nused > 0) {
+        get_value = H5O_msg_size_oh(file, ohdr, H5O_EFL_ID, &dset->shared->dcpl_cache.efl, 0);
+        if (get_value == 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of external file link message")
+        ret_value += get_value;
+    }
 
     /* Modification Time message size */
     if(H5O_HDR_STORE_TIMES & H5O_OH_GET_FLAGS(ohdr)) {
@@ -783,10 +811,14 @@ H5D__calculate_minimum_header_size(H5F_t *file, H5D_t *dset, H5O_t *ohdr)
         if(H5O_OH_GET_VERSION(ohdr) == 1) {
             /* v1 object headers store modification time as a message */
             time_t mtime;
-            ret_value += H5O_msg_size_oh(file, ohdr, H5O_MTIME_NEW_ID, &mtime, 0);
+            get_value = H5O_msg_size_oh(file, ohdr, H5O_MTIME_NEW_ID, &mtime, 0);
+            if (get_value == 0)
+                HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, 0, "can't get size of modification time message")
+            ret_value += get_value;
         }
     }
 
+done:
     FUNC_LEAVE_NOAPI(ret_value);
 } /* H5D__calculate_minimum_header_size */
 
@@ -822,6 +854,8 @@ H5D__prepare_minimized_oh(H5F_t *file, H5D_t *dset, H5O_loc_t *oloc)
         HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "can't instantiate object header")
 
     ohdr_size = H5D__calculate_minimum_header_size(file, dset, oh);
+    if (ohdr_size == 0)
+       HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "computed header size is invalid")
 
     if(H5O__apply_ohdr(file, oh, dset->shared->dcpl_id, ohdr_size, (size_t)1, oloc) == FAIL)
         HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "can't apply object header to file")
diff --git a/test/links.c b/test/links.c
index 4951038..b09ddb1 100644
--- a/test/links.c
+++ b/test/links.c
@@ -547,7 +547,7 @@ cklinks(hid_t fapl, hbool_t new_format)
     if(H5Oget_info_by_name2(file, "grp1/hard", &oinfo2, H5O_INFO_BASIC, H5P_DEFAULT) < 0) FAIL_STACK_ERROR
     if(H5O_TYPE_DATASET != oinfo2.type) {
 	H5_FAILED();
-	printf("    %d: Unexpected object type should have been a dataset\n", __LINE__);
+	HDprintf("    %d: Unexpected object type should have been a dataset\n", __LINE__);
 	TEST_ERROR
     } /* end if */
     if(H5F_addr_ne(oinfo1.addr, oinfo2.addr)) {
@@ -582,7 +582,7 @@ cklinks(hid_t fapl, hbool_t new_format)
     if(H5Oget_info_by_name2(file, "grp1/soft", &oinfo2, H5O_INFO_BASIC, H5P_DEFAULT) < 0) FAIL_STACK_ERROR
     if(H5O_TYPE_DATASET != oinfo2.type) {
 	H5_FAILED();
-	printf("    %d: Unexpected object type should have been a dataset\n", __LINE__);
+	HDprintf("    %d: Unexpected object type should have been a dataset\n", __LINE__);
 	TEST_ERROR
     } /* end if */
     if(H5F_addr_ne(oinfo1.addr, oinfo2.addr)) {
@@ -611,12 +611,12 @@ cklinks(hid_t fapl, hbool_t new_format)
     if(H5Lget_info(file, "grp1/dangle", &linfo2, H5P_DEFAULT) < 0) FAIL_STACK_ERROR
     if(H5L_TYPE_SOFT != linfo2.type) {
 	H5_FAILED();
-	printf("    %d: Unexpected object type should have been a symbolic link\n", __LINE__);
+	HDprintf("    %d: Unexpected object type should have been a symbolic link\n", __LINE__);
 	TEST_ERROR
     } /* end if */
     if(H5Lget_val(file, "grp1/dangle", linkval, sizeof linkval, H5P_DEFAULT) < 0) {
 	H5_FAILED();
-	printf("    %d: Can't retrieve link value\n", __LINE__);
+	HDprintf("    %d: Can't retrieve link value\n", __LINE__);
 	TEST_ERROR
     } /* end if */
     if(HDstrcmp(linkval, "foobar")) {
@@ -638,12 +638,12 @@ cklinks(hid_t fapl, hbool_t new_format)
     if(H5Lget_info(file, "grp1/recursive", &linfo2, H5P_DEFAULT) < 0) FAIL_STACK_ERROR
     if(H5L_TYPE_SOFT != linfo2.type) {
 	H5_FAILED();
-	printf("    %d: Unexpected object type should have been a symbolic link\n", __LINE__);
+	HDprintf("    %d: Unexpected object type should have been a symbolic link\n", __LINE__);
 	TEST_ERROR
     } /* end if */
     if(H5Lget_val(file, "grp1/recursive", linkval, sizeof linkval, H5P_DEFAULT) < 0) {
 	H5_FAILED();
-	printf("    %d: Can't retrieve link value\n", __LINE__);
+	HDprintf("    %d: Can't retrieve link value\n", __LINE__);
 	TEST_ERROR
     } /* end if */
     if(HDstrcmp(linkval, "/grp1/recursive")) {
@@ -708,7 +708,7 @@ ck_new_links(hid_t fapl, hbool_t new_format)
     /* Check hard links */
     if(H5O_TYPE_DATASET != oi_hard1.type || H5O_TYPE_DATASET != oi_hard2.type) {
 	H5_FAILED();
-	printf("    %d: Unexpected object type should have been a dataset\n", __LINE__);
+	HDprintf("    %d: Unexpected object type should have been a dataset\n", __LINE__);
 	TEST_ERROR
     }
     if(H5F_addr_ne(oi_dset.addr, oi_hard1.addr) || H5F_addr_ne(oi_dset.addr, oi_hard2.addr)) {
@@ -2604,7 +2604,7 @@ external_link_toomany(hid_t fapl, hbool_t new_format)
     } H5E_END_TRY;
     if (gid >= 0) {
 	H5_FAILED();
-	printf("%d:    Should have failed for sequence of too many nested links.", __LINE__);
+	HDprintf("%d:    Should have failed for sequence of too many nested links.", __LINE__);
 	goto error;
     }
 
@@ -14916,11 +14916,11 @@ main(void)
 
     for (minimize_dset_oh = 0; minimize_dset_oh <= 1; minimize_dset_oh++) {
         if (minimize_dset_oh) {
-            printf("\n-Testing with minimzed dataset object headers-\n");
+            HDprintf("\n-Testing with minimzed dataset object headers-\n");
             dcpl_g = H5Pcreate(H5P_DATASET_CREATE);
             if (0 > dcpl_g) TEST_ERROR
         } else {
-            printf("\n-Testing with unminimzed dataset object headers-\n");
+            HDprintf("\n-Testing with unminimzed dataset object headers-\n");
             dcpl_g = H5P_DEFAULT;
         }
 
@@ -14930,10 +14930,10 @@ main(void)
             /* Check for FAPL to use */
             if(new_format) {
                 my_fapl = fapl2;
-                printf("\n--Testing with 'new format'--\n");
+                HDprintf("\n--Testing with 'new format'--\n");
             } else {
                 my_fapl = fapl;
-                printf("\n--Testing with 'old format'--\n");
+                HDprintf("\n--Testing with 'old format'--\n");
             }
 
             /* always enter tests without external cache */
@@ -14976,12 +14976,12 @@ main(void)
                 if(efc) {
                     if(H5Pset_elink_file_cache_size(my_fapl, 8) < 0)
                         TEST_ERROR
-                    printf("\n---Testing with external file cache---\n");
+                    HDprintf("\n---Testing with external file cache---\n");
                 } /* end if */
                 else {
                     if(H5Pset_elink_file_cache_size(my_fapl, 0) < 0)
                         TEST_ERROR
-                    printf("\n---Testing without external file cache---\n");
+                    HDprintf("\n---Testing without external file cache---\n");
                 } /* end else */
 
                 nerrors += external_link_root(my_fapl, new_format) < 0 ? 1 : 0;
@@ -15107,11 +15107,11 @@ main(void)
 
     /* Results */
     if(nerrors) {
-        printf("***** %d LINK TEST%s FAILED! *****\n",
+        HDprintf("***** %d LINK TEST%s FAILED! *****\n",
                 nerrors, 1 == nerrors ? "" : "S");
         HDexit(EXIT_FAILURE);
     }
-    printf("All link tests passed.\n");
+    HDprintf("All link tests passed.\n");
 
     /* clean up symlink created by external link tests */
     HDremove(SYMLINK1);