Skip to content

Commit

Permalink
Avoid aligned access for references by decoding into temporary buffer…
Browse files Browse the repository at this point in the history 
… and then copying the result into the actual buffer. Update test to be more thorough with using compound datatype fields everywhere. (HDFGroup#206)
  • Loading branch information
@qkoziol @jhendersonHDF
qkoziol authored and jhendersonHDF committed Jan 20, 2021
1 parent 6d6eeb4 commit b9d7b85
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 42 deletions.
4 changes: 3 additions & 1 deletion src/H5Rint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1041,12 +1041,13 @@ H5R__decode(const unsigned char *buf, size_t *nbytes, H5R_ref_priv_t *ref)

FUNC_ENTER_PACKAGE

/* Sanity checks */
HDassert(buf);
HDassert(nbytes);
HDassert(ref);
buf_size = *nbytes;

/* Don't decode if buffer size isn't big enough */
buf_size = *nbytes;
if (buf_size < H5R_ENCODE_HEADER_SIZE)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "Buffer size is too small")

Expand Down Expand Up @@ -1107,6 +1108,7 @@ H5R__decode(const unsigned char *buf, size_t *nbytes, H5R_ref_priv_t *ref)
H5R_LOG_DEBUG("Decoded reference, filename=%s, obj_addr=%s, encode size=%u", ref->info.obj.filename,
H5R__print_token(ref->info.obj.token), ref->encode_size);

/* Set output info */
*nbytes = decode_size;

done:
Expand Down
17 changes: 11 additions & 6 deletions src/H5Tref.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
H5F_t * src_f = NULL;
hid_t file_id = H5I_INVALID_HID;
H5R_ref_priv_t *dst_ref = (H5R_ref_priv_t *)dst_buf;
H5R_ref_priv_t tmp_ref; /* Temporary reference to decode into */
herr_t ret_value = SUCCEED;

FUNC_ENTER_STATIC
Expand All @@ -599,6 +600,7 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
HDassert(src_size);
HDassert(dst_buf);
HDassert(dst_size == H5T_REF_MEM_SIZE);
HDcompile_assert(sizeof(*dst_ref) == sizeof(tmp_ref));

/* Memory-to-memory conversion to support vlen conversion */
if (NULL == src_file) {
Expand All @@ -624,21 +626,21 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid VOL object")

/* Make sure reference buffer is correctly initialized */
HDmemset(dst_buf, 0, dst_size);
HDmemset(&tmp_ref, 0, sizeof(tmp_ref));

switch (src_type) {
case H5R_OBJECT1: {
size_t token_size = H5F_SIZEOF_ADDR(src_f);

if (H5R__create_object((const H5O_token_t *)src_buf, token_size, dst_ref) < 0)
if (H5R__create_object((const H5O_token_t *)src_buf, token_size, &tmp_ref) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCREATE, FAIL, "unable to create object reference")
} break;

case H5R_DATASET_REGION1: {
const struct H5Tref_dsetreg *src_reg = (const struct H5Tref_dsetreg *)src_buf;
size_t token_size = H5F_SIZEOF_ADDR(src_f);

if (H5R__create_region(&src_reg->token, token_size, src_reg->space, dst_ref) < 0)
if (H5R__create_region(&src_reg->token, token_size, src_reg->space, &tmp_ref) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCREATE, FAIL, "unable to create region reference")

/* create_region creates its internal copy of the space */
Expand All @@ -655,7 +657,7 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
case H5R_OBJECT2:
case H5R_ATTR:
/* Decode reference */
if (H5R__decode((const unsigned char *)src_buf, &src_size, dst_ref) < 0)
if (H5R__decode((const unsigned char *)src_buf, &src_size, &tmp_ref) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "Cannot decode reference")
break;

Expand All @@ -667,17 +669,20 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
} /* end switch */

/* If no filename set, this is not an external reference */
if (NULL == H5R_REF_FILENAME(dst_ref)) {
if (NULL == H5R_REF_FILENAME(&tmp_ref)) {
/* TODO temporary hack to retrieve file object */
if ((file_id = H5F_get_file_id(src_file, H5I_FILE, FALSE)) < 0)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a file or file object")

/* Attach loc ID to reference and hold reference to it, this is a
* user exposed reference so set app_ref to TRUE. */
if (H5R__set_loc_id(dst_ref, file_id, TRUE, TRUE) < 0)
if (H5R__set_loc_id(&tmp_ref, file_id, TRUE, TRUE) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "unable to attach location id to reference")
} /* end if */

/* Set output info */
HDmemcpy(dst_ref, &tmp_ref, sizeof(tmp_ref));

done:
if ((file_id != H5I_INVALID_HID) && (H5I_dec_ref(file_id) < 0))
HDONE_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "unable to decrement refcount on location id")
Expand Down
67 changes: 32 additions & 35 deletions test/trefer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ test_reference_vlen_obj(void)
hsize_t vl_dims[] = {1};
hid_t dapl_id; /* Dataset access property list */
H5R_ref_t *wbuf, /* buffer to write to disk */
*rbuf; /* buffer read from disk */
*rbuf = NULL; /* buffer read from disk */
unsigned * ibuf, *obuf;
unsigned i, j; /* Counters */
H5O_type_t obj_type; /* Object type */
Expand All @@ -578,7 +578,6 @@ test_reference_vlen_obj(void)

/* Allocate write & read buffers */
wbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
rbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
ibuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);
obuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);

Expand Down Expand Up @@ -832,8 +831,6 @@ test_reference_cmpnd_obj(void)
hsize_t dims1[] = {SPACE1_DIM1};
hsize_t cmpnd_dims[] = {1};
hid_t dapl_id; /* Dataset access property list */
H5R_ref_t *wbuf, /* buffer to write to disk */
*rbuf; /* buffer read from disk */
unsigned * ibuf, *obuf;
unsigned i, j; /* Counters */
H5O_type_t obj_type; /* Object type */
Expand All @@ -844,8 +841,6 @@ test_reference_cmpnd_obj(void)
MESSAGE(5, ("Testing Object Reference Functions within compound type\n"));

/* Allocate write & read buffers */
wbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
rbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
ibuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);
obuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);

Expand Down Expand Up @@ -946,39 +941,38 @@ test_reference_cmpnd_obj(void)
dataset = H5Dcreate2(fid1, "Dataset3", tid1, sid1, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
CHECK(dataset, H5I_INVALID_HID, "H5Dcreate2");

/* Reset buffer for writing */
HDmemset(&cmpnd_wbuf, 0, sizeof(cmpnd_wbuf));

/* Create reference to dataset */
ret = H5Rcreate_object(fid1, "/Group1/Dataset1", H5P_DEFAULT, &wbuf[0]);
ret = H5Rcreate_object(fid1, "/Group1/Dataset1", H5P_DEFAULT, &cmpnd_wbuf.ref0);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[0], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref0, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_DATASET, "H5Rget_obj_type3");

/* Create reference to dataset */
ret = H5Rcreate_object(fid1, "/Group1/Dataset2", H5P_DEFAULT, &wbuf[1]);
ret = H5Rcreate_object(fid1, "/Group1/Dataset2", H5P_DEFAULT, &cmpnd_wbuf.ref1);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[1], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref1, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_DATASET, "H5Rget_obj_type3");

/* Create reference to group */
ret = H5Rcreate_object(fid1, "/Group1", H5P_DEFAULT, &wbuf[2]);
ret = H5Rcreate_object(fid1, "/Group1", H5P_DEFAULT, &cmpnd_wbuf.ref2);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[2], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref2, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_GROUP, "H5Rget_obj_type3");

/* Create reference to named datatype */
ret = H5Rcreate_object(fid1, "/Group1/Datatype1", H5P_DEFAULT, &wbuf[3]);
ret = H5Rcreate_object(fid1, "/Group1/Datatype1", H5P_DEFAULT, &cmpnd_wbuf.ref3);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[3], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref3, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_NAMED_DATATYPE, "H5Rget_obj_type3");

/* Store dimensions */
cmpnd_wbuf.ref0 = wbuf[0];
cmpnd_wbuf.ref1 = wbuf[1];
cmpnd_wbuf.ref2 = wbuf[2];
cmpnd_wbuf.ref3 = wbuf[3];
cmpnd_wbuf.dim_idx = SPACE1_DIM1;

/* Write selection to disk */
Expand Down Expand Up @@ -1017,17 +1011,13 @@ test_reference_cmpnd_obj(void)
CHECK(ret, FAIL, "H5Dread");

VERIFY(cmpnd_rbuf.dim_idx, SPACE1_DIM1, "H5Dread");
rbuf[0] = cmpnd_rbuf.ref0;
rbuf[1] = cmpnd_rbuf.ref1;
rbuf[2] = cmpnd_rbuf.ref2;
rbuf[3] = cmpnd_rbuf.ref3;

/* Close datatype */
ret = H5Tclose(tid1);
CHECK(ret, FAIL, "H5Tclose");

/* Open dataset object */
dset2 = H5Ropen_object(&rbuf[0], H5P_DEFAULT, dapl_id);
dset2 = H5Ropen_object(&cmpnd_rbuf.ref0, H5P_DEFAULT, dapl_id);
CHECK(dset2, H5I_INVALID_HID, "H5Ropen_object");

/* Check information in referenced dataset */
Expand All @@ -1049,15 +1039,15 @@ test_reference_cmpnd_obj(void)
CHECK(ret, FAIL, "H5Dclose");

/* Open group object. GAPL isn't supported yet. But it's harmless to pass in */
group = H5Ropen_object(&rbuf[2], H5P_DEFAULT, H5P_DEFAULT);
group = H5Ropen_object(&cmpnd_rbuf.ref2, H5P_DEFAULT, H5P_DEFAULT);
CHECK(group, H5I_INVALID_HID, "H5Ropen_object");

/* Close group */
ret = H5Gclose(group);
CHECK(ret, FAIL, "H5Gclose");

/* Open datatype object. TAPL isn't supported yet. But it's harmless to pass in */
tid1 = H5Ropen_object(&rbuf[3], H5P_DEFAULT, H5P_DEFAULT);
tid1 = H5Ropen_object(&cmpnd_rbuf.ref3, H5P_DEFAULT, H5P_DEFAULT);
CHECK(tid1, H5I_INVALID_HID, "H5Ropen_object");

/* Verify correct datatype */
Expand Down Expand Up @@ -1088,18 +1078,25 @@ test_reference_cmpnd_obj(void)
CHECK(ret, FAIL, "H5Fclose");

/* Destroy references */
for (j = 0; j < SPACE1_DIM1; j++) {
ret = H5Rdestroy(&wbuf[j]);
CHECK(ret, FAIL, "H5Rdestroy");
}
for (j = 0; j < SPACE1_DIM1; j++) {
ret = H5Rdestroy(&rbuf[j]);
CHECK(ret, FAIL, "H5Rdestroy");
}
ret = H5Rdestroy(&cmpnd_wbuf.ref0);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_wbuf.ref1);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_wbuf.ref2);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_wbuf.ref3);
CHECK(ret, FAIL, "H5Rdestroy");

ret = H5Rdestroy(&cmpnd_rbuf.ref0);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_rbuf.ref1);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_rbuf.ref2);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_rbuf.ref3);
CHECK(ret, FAIL, "H5Rdestroy");

/* Free memory buffers */
HDfree(wbuf);
HDfree(rbuf);
HDfree(ibuf);
HDfree(obuf);
} /* test_reference_cmpnd_obj() */
Expand Down

0 comments on commit b9d7b85

Please sign in to comment.