From e7037ad07213833582cf066b05f6dbf52c626ce6 Mon Sep 17 00:00:00 2001 From: Raymond Lu Date: Mon, 21 Jul 2008 11:50:22 -0500 Subject: [svn-r15390] Last round of checkin introduced a bug. This checkin corrected it and made a few minor changes. Last round of check in fixed the problem when an attribute was opened twice and data was written with one of the handles, the file didn't have the data. It happened because each handle had its own object structure, and the empty one overwrote the data with fill value. This is fixed by making some attribute information like the data be shared in the attribute structure. Tested on smirom, kagiso, and linew. --- c++/test/testhdf5.cpp | 2 - src/H5A.c | 4 +- src/H5Aint.c | 8 +++- src/H5Oattr.c | 2 - src/H5Oattribute.c | 114 ++++++++++++++++++++++++++++---------------------- test/testhdf5.c | 2 - 6 files changed, 70 insertions(+), 62 deletions(-) diff --git a/c++/test/testhdf5.cpp b/c++/test/testhdf5.cpp index a09e2d8..02b427e 100644 --- a/c++/test/testhdf5.cpp +++ b/c++/test/testhdf5.cpp @@ -76,10 +76,8 @@ main(int argc, char *argv[]) AddTest("file", test_file, cleanup_file, "File I/O Operations", NULL); // testing dataspace functionalities in th5s.cpp AddTest("h5s", test_h5s, cleanup_h5s, "Dataspaces", NULL); -#ifdef TMP // testing attribute functionalities in tattr.cpp AddTest("attr", test_attr, cleanup_attr, "Attributes", NULL); -#endif // testing reference functionalities in trefer.cpp AddTest("reference", test_reference, cleanup_reference, "References", NULL); // testing variable-length strings in tvlstr.cpp diff --git a/src/H5A.c b/src/H5A.c index 7675b90..aec538f 100644 --- a/src/H5A.c +++ b/src/H5A.c @@ -2417,14 +2417,12 @@ done: herr_t H5A_close(H5A_t *attr) { - H5O_t *oh = NULL; /* Pointer to attr's object header */ - unsigned free_failed = FALSE; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI(H5A_close, FAIL) HDassert(attr); - HDassert(attr->shared>0); + HDassert(attr->shared); /* Close the object's symbol-table entry */ if(attr->obj_opened && (H5O_close(&(attr->shared->oloc)) < 0)) diff --git a/src/H5Aint.c b/src/H5Aint.c index 1ff4c43..bd4edfc 100644 --- a/src/H5Aint.c +++ b/src/H5Aint.c @@ -140,10 +140,14 @@ H5A_compact_build_table_cb(H5O_t UNUSED *oh, H5O_mesg_t *mesg/*in,out*/, size_t n = MAX(1, 2 * udata->atable->nattrs); H5A_t **table = (H5A_t **)H5FL_SEQ_CALLOC(H5A_t_ptr, n); - /* Use attribute functions for operation */ + /* Use attribute functions for operation to manage memory properly */ for(i=0; iatable->nattrs; i++) { table[i] = (H5A_t *)H5FL_CALLOC(H5A_t); - HDmemcpy(&(table[i]), &(udata->atable->attrs[i]), sizeof(H5A_t*)); + + if(NULL == H5A_copy(table[i], udata->atable->attrs[i])) + HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, H5_ITER_ERROR, "can't copy attribute") + if(H5A_close(udata->atable->attrs[i]) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, H5_ITER_ERROR, "can't close attribute") } if(udata->atable->nattrs) diff --git a/src/H5Oattr.c b/src/H5Oattr.c index bf4c521..4bfeb3d 100644 --- a/src/H5Oattr.c +++ b/src/H5Oattr.c @@ -468,8 +468,6 @@ H5O_attr_size(const H5F_t UNUSED *f, const void *_mesg) herr_t H5O_attr_reset(void *_mesg) { - H5A_t *attr = (H5A_t *)_mesg; - FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5O_attr_reset) FUNC_LEAVE_NOAPI(SUCCEED) diff --git a/src/H5Oattribute.c b/src/H5Oattribute.c index 249437b..701dd05 100644 --- a/src/H5Oattribute.c +++ b/src/H5Oattribute.c @@ -40,6 +40,8 @@ #include "H5MMprivate.h" /* Memory management */ #include "H5Opkg.h" /* Object headers */ #include "H5SMprivate.h" /* Shared Object Header Messages */ +#include "H5Iprivate.h" /* IDs */ +#include "H5Fprivate.h" /* File */ /****************/ @@ -474,6 +476,8 @@ H5O_attr_open_by_name(const H5O_loc_t *loc, const char *name, hid_t dxpl_id) H5O_t *oh = NULL; /* Pointer to actual object header */ H5O_ainfo_t ainfo; /* Attribute information for object */ H5A_t *ret_value; /* Return value */ + H5A_t *exist_attr = NULL; /* Opened attribute object */ + htri_t found_open_attr = FALSE; /* Whether opened object is found */ FUNC_ENTER_NOAPI_NOINIT(H5O_attr_open_by_name) @@ -491,43 +495,40 @@ H5O_attr_open_by_name(const H5O_loc_t *loc, const char *name, hid_t dxpl_id) /* Clear error stack from not finding attribute info */ H5E_clear_stack(NULL); - /* Check for opening attribute with dense storage */ - if(H5F_addr_defined(ainfo.fheap_addr)) { - H5A_t *exist_attr = NULL; - htri_t found_open_attr = FALSE; - - /* If found the attribute is already opened, make a copy of it to share the - object information. If not, open attribute in dense storage */ - if((found_open_attr = H5O_attr_find_opened_attr(loc, &exist_attr, name)) < 0) - HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, NULL, "failed in finding opened attribute") - else if(found_open_attr == TRUE) { - if(NULL == (ret_value = H5A_copy(NULL, exist_attr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, NULL, "can't copy existing attribute") - } else if(NULL == (ret_value = H5A_dense_open(loc->file, dxpl_id, &ainfo, name))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, NULL, "can't open attribute") - } /* end if */ - else { - H5O_iter_opn_t udata; /* User data for callback */ - H5O_mesg_operator_t op; /* Wrapper for operator */ - - /* Set up user data for callback */ - udata.name = name; - udata.attr = NULL; - - /* Iterate over attributes, to locate correct one to open */ - op.op_type = H5O_MESG_OP_LIB; - op.u.lib_op = H5O_attr_open_cb; - if(H5O_msg_iterate_real(loc->file, oh, H5O_MSG_ATTR, &op, &udata, dxpl_id) < 0) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, NULL, "error updating attribute") - - /* Check that we found the attribute */ - if(!udata.attr) - HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, NULL, "can't locate attribute") - - /* Get attribute opened from object header */ - HDassert(udata.attr); - ret_value = udata.attr; - } /* end else */ + /* If found the attribute is already opened, make a copy of it to share the + object information. If not, open attribute as a new object */ + if((found_open_attr = H5O_attr_find_opened_attr(loc, &exist_attr, name)) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, NULL, "failed in finding opened attribute") + else if(found_open_attr == TRUE) { + if(NULL == (ret_value = H5A_copy(NULL, exist_attr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, NULL, "can't copy existing attribute") + } else { + if(H5F_addr_defined(ainfo.fheap_addr)) { /* Open attribute with dense storage */ + if(NULL == (ret_value = H5A_dense_open(loc->file, dxpl_id, &ainfo, name))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, NULL, "can't open attribute") + } else { /* Open attribute in compact storage */ + H5O_iter_opn_t udata; /* User data for callback */ + H5O_mesg_operator_t op; /* Wrapper for operator */ + + /* Set up user data for callback */ + udata.name = name; + udata.attr = NULL; + + /* Iterate over attributes, to locate correct one to open */ + op.op_type = H5O_MESG_OP_LIB; + op.u.lib_op = H5O_attr_open_cb; + if(H5O_msg_iterate_real(loc->file, oh, H5O_MSG_ATTR, &op, &udata, dxpl_id) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, NULL, "error updating attribute") + + /* Check that we found the attribute */ + if(!udata.attr) + HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, NULL, "can't locate attribute") + + /* Get attribute opened from object header */ + HDassert(udata.attr); + ret_value = udata.attr; + } /* end else */ + } done: if(oh && H5AC_unprotect(loc->file, dxpl_id, H5AC_OHDR, loc->addr, oh, H5AC__NO_FLAGS_SET) < 0) @@ -595,8 +596,8 @@ H5O_attr_open_by_idx(const H5O_loc_t *loc, H5_index_t idx_type, H5_iter_order_t order, hsize_t n, hid_t dxpl_id) { H5A_attr_iter_op_t attr_op; /* Attribute operator */ - H5A_t *exist_attr = NULL; - htri_t found_open_attr = FALSE; + H5A_t *exist_attr = NULL; /* Opened attribute object */ + htri_t found_open_attr = FALSE; /* Whether opened object is found */ H5A_t *ret_value = NULL; /* Return value */ H5O_t *oh = NULL; /* Object header */ H5O_ainfo_t ainfo; /* Attribute information for object */ @@ -625,10 +626,9 @@ H5O_attr_open_by_idx(const H5O_loc_t *loc, H5_index_t idx_type, /* Clear error stack from not finding attribute info */ H5E_clear_stack(NULL); - /* If the opened attribute is in dense storage, find out whether it has already been - * opened. If it has, close the object and make a copy of the already opened object - * to share the object info. */ - if(H5F_addr_defined(ainfo.fheap_addr) && ret_value) { + /* Find out whether it has already been opened. If it has, close the object + * and make a copy of the already opened object to share the object info. */ + if(ret_value) { if((found_open_attr = H5O_attr_find_opened_attr(loc, &exist_attr, ret_value->shared->name)) < 0) HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, NULL, "failed in finding opened attribute") @@ -673,14 +673,18 @@ htri_t H5O_attr_find_opened_attr(const H5O_loc_t *loc, H5A_t **attr, const char* { htri_t ret_value = FALSE; int num_open_attr = 0; - ssize_t name_len = 0; hid_t *attr_id_list = NULL; + unsigned long loc_fnum, attr_fnum; int i; FUNC_ENTER_NOAPI_NOINIT(H5O_attr_find_opened_attr) + /* Get file serial number for the location of attribute */ + if(H5F_get_fileno(loc->file, &loc_fnum) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_BADVALUE, FAIL, "can't get file serial number") + /* Count all opened attributes */ - if((num_open_attr = H5F_get_obj_count(loc->file, H5F_OBJ_ATTR)) < 0) + if((num_open_attr = H5F_get_obj_count(loc->file, H5F_OBJ_ATTR | H5F_OBJ_LOCAL)) < 0) HGOTO_ERROR(H5E_ATTR, H5E_CANTCOUNT, FAIL, "can't get number of opened attributes") /* Find out whether the attribute has been opened */ @@ -688,16 +692,23 @@ htri_t H5O_attr_find_opened_attr(const H5O_loc_t *loc, H5A_t **attr, const char* attr_id_list = (hid_t*)H5MM_malloc(num_open_attr*sizeof(hid_t)); /* Retrieve the IDs of all opened attributes */ - if(H5F_get_obj_ids(loc->file, H5F_OBJ_ATTR, num_open_attr, attr_id_list) < 0) + if(H5F_get_obj_ids(loc->file, H5F_OBJ_ATTR | H5F_OBJ_LOCAL, num_open_attr, attr_id_list) < 0) HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't IDs of opened attributes") for(i=0; ishared->name) && loc->addr == - (*attr)->shared->oloc.addr) { + /* Get file serial number for attribute */ + if(H5F_get_fileno((*attr)->shared->oloc.file, &attr_fnum) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_BADVALUE, FAIL, "can't get file serial number") + + /* Verify whether it's the right object. The attribute name, object address + * to which the attribute is attached, and file serial number should all + * match. */ + if(!strcmp(name_to_open, (*attr)->shared->name) && + loc->addr == (*attr)->shared->oloc.addr && + loc_fnum == attr_fnum) { ret_value = TRUE; break; } @@ -1304,7 +1315,8 @@ done: * the attribute is already opened, use the opened message * to insert. If not, still use the message in the attribute * table. This will guarantee that the attribute message is - * shared. + * shared between the object in metadata cache and the opened + * object. *------------------------------------------------------------------------- */ static herr_t diff --git a/test/testhdf5.c b/test/testhdf5.c index 93c3b58..95ec90c 100644 --- a/test/testhdf5.c +++ b/test/testhdf5.c @@ -55,10 +55,8 @@ main(int argc, char *argv[]) AddTest("objects", test_h5o, cleanup_h5o, "Generic Object Functions", NULL); AddTest("h5s", test_h5s, cleanup_h5s, "Dataspaces", NULL); AddTest("coords", test_coords, cleanup_coords, "Dataspace coordinates", NULL); -#ifdef TMP AddTest("sohm", test_sohm, cleanup_sohm, "Shared Object Header Messages", NULL); AddTest("attr", test_attr, cleanup_attr, "Attributes", NULL); -#endif AddTest("select", test_select, cleanup_select, "Selections", NULL); AddTest("time", test_time, cleanup_time, "Time Datatypes", NULL); AddTest("reference", test_reference, cleanup_reference, "References", NULL); -- cgit v0.12