From db9cc49a6bf77bf5125bbabb833c9428ada44b52 Mon Sep 17 00:00:00 2001 From: Jacob Smith Date: Fri, 21 Dec 2018 15:45:56 -0600 Subject: Add error checking to the minimized dset header size calculation. Update printf->HDprintf statements. --- src/H5Dint.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++-------------- test/links.c | 32 +++++++++++++++--------------- 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); -- cgit v0.12