Fix two assertion failures in hyperslab code #5355
Conversation
jhendersonHDF
added
Priority - 3. Low 🔽
jhendersonHDF
requested review from
lrknox,
derobins,
byrnHDF,
fortnern,
qkoziol,
vchoi-hdfgroup,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
jhendersonHDF
force-pushed
the
hyper_span_copy_bug
branch
2 times, most recently
from
2e9c39d to
d9591a8
Compare
jhendersonHDF
force-pushed
the
hyper_span_copy_bug
branch
from
d9591a8 to
fc50fca
Compare
|
|
||
| FUNC_ENTER_NOAPI(FAIL) | ||
|
|
||
| /* Check args */ | ||
| assert(dst); | ||
| assert(src); | ||
|
|
||
| /* Release the current selection */ | ||
| if (H5S_SELECT_RELEASE(dst) < 0) |
There was a problem hiding this comment.
Part of this PR was fixing H5Scopy() to return failure for datasets with a NULL extent (rank value of 0). However, the previous code didn't really account for failure and would release the selection in the destination space before determining if the selection copy portion actually succeeded or not. This could result in segfaults later on when the application tries to close the destination dataspace. This reworks things so the copy is made into a temporary dataspace and only releases the selection in the destination dataspace once things have succeeded.
| @@ -3215,11 +3217,11 @@ H5S__hyper_copy(H5S_t *dst, const H5S_t *src, bool share_selection) | |||
| assert(dst); | |||
|
|
|||
| /* Allocate space for the hyperslab selection information */ | |||
| if (NULL == (dst->select.sel_info.hslab = H5FL_MALLOC(H5S_hyper_sel_t))) | |||
There was a problem hiding this comment.
The code here and below was directly manipulating the destination dataspace, which could leave things in a bad state if failure occurs. This function had to be reworked to use the local dst_hslab and only manipulate the destination dataspace after things have succeeded.
| /* Copy the hyperslab span information */ | ||
| dst->select.sel_info.hslab->span_lst = | ||
| H5S__hyper_copy_span(src->select.sel_info.hslab->span_lst, src->extent.rank); |
There was a problem hiding this comment.
Code wasn't previously checking for errors here, which would hide problems when trying to copy a dataspace with an extent of 0 dims.
No description provided.