From 95983cbdfe74dc991c190cb04e2ff76202cac31c Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Tue, 31 Mar 2015 15:20:10 -0500 Subject: [svn-r26680] Implement minor suggestions from 3/26/15 code review. Note make check still fails in h5dump test (unrelated to this checkin). Tested: ummon --- src/H5Dvirtual.c | 39 +++++++++++++++++++-------------------- src/H5S.c | 6 +----- src/H5Shyper.c | 10 ++++------ src/H5Sselect.c | 4 +--- 4 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/H5Dvirtual.c b/src/H5Dvirtual.c index 698aea8..3643c53 100644 --- a/src/H5Dvirtual.c +++ b/src/H5Dvirtual.c @@ -69,7 +69,7 @@ static herr_t H5D__virtual_write(H5D_io_info_t *io_info, static herr_t H5D__virtual_flush(H5D_t *dset, hid_t dxpl_id); /* Other functions */ -static htri_t H5D__virtual_open_source_dset(const H5D_t *vdset, +static herr_t H5D__virtual_open_source_dset(const H5D_t *vdset, H5O_storage_virtual_ent_t *virtual_ent, hid_t dxpl_id); @@ -191,14 +191,14 @@ done: * *------------------------------------------------------------------------- */ -static htri_t +static herr_t H5D__virtual_open_source_dset(const H5D_t *vdset, H5O_storage_virtual_ent_t *virtual_ent, hid_t dxpl_id) { H5F_t *src_file = NULL; /* Source file */ hbool_t src_file_open = FALSE; /* Whether we have opened and need to close src_file */ H5G_loc_t src_root_loc; /* Object location of source file root group */ - htri_t ret_value = TRUE; /* Return value */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_STATIC @@ -207,7 +207,8 @@ H5D__virtual_open_source_dset(const H5D_t *vdset, HDassert(!virtual_ent->source_dset); /* Get dapl and fapl from current (virtual dataset) location? VDSINC */ - /* Write code to check if these exist and return FALSE otherwise VDSINC */ + /* Write code to check if these exist and return without opening dset + * otherwise VDSINC */ /* Check if we need to open the source file */ if(HDstrcmp(virtual_ent->source_file_name, ".")) { @@ -447,7 +448,6 @@ H5D__virtual_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, H5S_t *projected_mem_space = NULL; /* Memory space for selection in a single mapping */ H5S_t *projected_src_space = NULL; /* File space for selection in a single source dataset */ hssize_t select_nelmts; /* Number of elements in selection */ - htri_t opened_dset; /* Whether we opened the source dataset */ size_t i; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ @@ -479,18 +479,16 @@ H5D__virtual_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, /* Only perform I/O if there are any elements */ if(select_nelmts > 0) { /* Open source dataset */ - if(!storage->list[i].source_dset) { + if(!storage->list[i].source_dset) /* Try to open dataset */ - if((opened_dset = H5D__virtual_open_source_dset(io_info->dset, &storage->list[i], io_info->dxpl_id)) < 0) + if(H5D__virtual_open_source_dset(io_info->dset, &storage->list[i], io_info->dxpl_id) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTOPENOBJ, FAIL, "unable to open source dataset") - if(opened_dset) - /* Sanity check that the source space has been patched by now */ - HDassert(storage->list[i].source_space_status == H5O_VIRTUAL_STATUS_CORRECT); - } /* end if */ - /* Check if source dataset is open */ if(storage->list[i].source_dset) { + /* Sanity check that the source space has been patched by now */ + HDassert(storage->list[i].source_space_status == H5O_VIRTUAL_STATUS_CORRECT); + /* Project intersection of file space and mapping virtual space onto * mapping source space */ if(H5S_select_project_intersection(storage->list[i].virtual_select, storage->list[i].source_select, file_space, &projected_src_space) < 0) @@ -516,6 +514,9 @@ H5D__virtual_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, projected_mem_space = NULL; } /* end for */ + /* Fill unmapped part of buffer with fill value. Keep track of total number + * elements written to memory buffer and assert that it == nelmts */ + done: /* Release allocated resources on failure */ if(projected_src_space) { @@ -554,7 +555,6 @@ H5D__virtual_write(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, H5S_t *projected_mem_space = NULL; /* Memory space for selection in a single mapping */ H5S_t *projected_src_space = NULL; /* File space for selection in a single source dataset */ hssize_t select_nelmts; /* Number of elements in selection */ - htri_t opened_dset; /* Whether we opened the source dataset */ size_t i; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ @@ -588,15 +588,15 @@ H5D__virtual_write(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, /* Open source dataset, fail if cannot open */ if(!storage->list[i].source_dset) { //VDSINC check all source datasets before any I/O - if((opened_dset = H5D__virtual_open_source_dset(io_info->dset, &storage->list[i], io_info->dxpl_id)) < 0) + if(H5D__virtual_open_source_dset(io_info->dset, &storage->list[i], io_info->dxpl_id) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTOPENOBJ, FAIL, "unable to open source dataset") - if(!opened_dset) + if(!storage->list[i].source_dset) HGOTO_ERROR(H5E_DATASET, H5E_CANTOPENOBJ, FAIL, "did not open source dataset") - - /* Sanity check that source space has been patched by now */ - HDassert(storage->list[i].source_space_status == H5O_VIRTUAL_STATUS_CORRECT); } /* end if */ + /* Sanity check that source space has been patched by now */ + HDassert(storage->list[i].source_space_status == H5O_VIRTUAL_STATUS_CORRECT); + /* Extend source dataset if necessary and there is an unlimited * dimension VDSINC */ /* Project intersection of file space and mapping virtual space onto @@ -654,8 +654,7 @@ H5D__virtual_flush(H5D_t UNUSED *dset, hid_t UNUSED dxpl_id) { FUNC_ENTER_STATIC_NOERR - /* Need to decide what to do here - flush only open datasets, try to flush - * all, or do nothing and rely on source datasets to flush themselves? */ + /* Flush only open datasets */ /* No-op for now VDSINC */ FUNC_LEAVE_NOAPI(SUCCEED) diff --git a/src/H5S.c b/src/H5S.c index 4d6ef76..717374c 100644 --- a/src/H5S.c +++ b/src/H5S.c @@ -479,7 +479,7 @@ H5Sextent_copy(hid_t dst_id,hid_t src_id) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a dataspace") /* Copy */ - if(H5S_extent_copy_real(&(dst->extent), &(src->extent), TRUE) < 0) + if(H5S_extent_copy(dst, src) < 0) HGOTO_ERROR(H5E_DATASPACE, H5E_CANTCOPY, FAIL, "can't copy extent") done: @@ -521,10 +521,6 @@ H5S_extent_copy(H5S_t *dst, const H5S_t *src) if(H5S_select_all(dst, FALSE) < 0) HGOTO_ERROR(H5E_DATASPACE, H5E_CANTDELETE, FAIL, "can't change selection") - /* Mark the destination space as no longer shared if it was before */ - if(H5O_msg_reset_share(H5O_SDSPACE_ID, dst) < 0) - HGOTO_ERROR(H5E_DATASPACE, H5E_CANTRESET, FAIL, "can't stop sharing dataspace") - done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5S_extent_copy() */ diff --git a/src/H5Shyper.c b/src/H5Shyper.c index 13573e1..51194f9 100644 --- a/src/H5Shyper.c +++ b/src/H5Shyper.c @@ -8963,10 +8963,8 @@ H5S__hyper_project_intersection(const H5S_t *src_space, const H5S_t *dst_space, /* Calculate proj_down_dims (note loop relies on unsigned i wrapping around) */ - proj_down_dims[proj_rank - 1] = proj_space->extent.size[proj_rank - 1]; - if(proj_rank > 1) - for(i = proj_rank - 2; i < proj_rank; i--) - proj_down_dims[i] = proj_space->extent.size[i] * proj_down_dims[i + 1]; + if(H5VM_array_down(proj_rank, proj_space->extent.size, proj_down_dims) < 0) + HGOTO_ERROR(H5E_DATASPACE, H5E_CANTSET, FAIL, "can't compute 'down' chunk size value") /* Remove current selection from proj_space */ if(H5S_SELECT_RELEASE(proj_space) < 0) @@ -9137,7 +9135,7 @@ H5S__hyper_project_intersection(const H5S_t *src_space, const H5S_t *dst_space, * the plane of the span currently being built (i.e. it's * finished being built) */ for(i = proj_rank - 1; ((i > 0) - && (((proj_off / proj_down_dims[i]) + && (((proj_off / proj_down_dims[i - 1]) % proj_space->extent.size[i]) != curr_span_dim[i - 1])); i--) { if(curr_span_tree[i]) { @@ -9158,7 +9156,7 @@ H5S__hyper_project_intersection(const H5S_t *src_space, const H5S_t *dst_space, } /* end if */ /* Update curr_span_dim */ - curr_span_dim[i - 1] = (proj_off / proj_down_dims[i]) % proj_space->extent.size[i]; + curr_span_dim[i - 1] = (proj_off / proj_down_dims[i - 1]) % proj_space->extent.size[i]; } /* end for */ /* Compute bounds for new span in lowest dimension */ diff --git a/src/H5Sselect.c b/src/H5Sselect.c index ed384e5..b62186b 100644 --- a/src/H5Sselect.c +++ b/src/H5Sselect.c @@ -2128,7 +2128,6 @@ H5S_select_project_intersection(const H5S_t *src_space, const H5S_t *dst_space, const H5S_t *src_intersect_space, H5S_t **new_space_ptr) { H5S_t *new_space = NULL; /* New dataspace constructed */ - unsigned i; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI(FAIL) @@ -2146,8 +2145,7 @@ H5S_select_project_intersection(const H5S_t *src_space, const H5S_t *dst_space, HGOTO_ERROR(H5E_DATASPACE, H5E_CANTCOPY, FAIL, "unable to copy destination space extent") /* Set offset to zeros */ - for(i = 0; i < new_space->extent.rank; i++) - new_space->select.offset[i] = 0; + (void)HDmemset(new_space->select.offset, 0, (size_t)new_space->extent.rank * sizeof(new_space->select.offset[0])); new_space->select.offset_changed = FALSE; /* If the intersecting space is "all", the intersection must be equal to the -- cgit v0.12