From 2f665b89ce8f1b28a4a8fbe40cf3f2f507e6b860 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Mon, 11 Dec 2006 20:49:59 -0500 Subject: [svn-r13047] Description: Add "attribute exists" internal routine to make verifying that an attribute with the same name doesn't already exist easier. Tweak "trace" script to produce more whitespace in H5TRACE macros, in order to make them easier to read. Minor other whitespace cleanups Tested on: Linux/32 2.6 (chicago) Linux/64 2.6 (chicago2) --- bin/trace | 4 +- src/H5A.c | 75 ++++++----------------------- src/H5Adense.c | 65 ++++++++++++++++++++++++- src/H5Apkg.h | 4 +- src/H5B2.c | 16 +++--- src/H5Oattribute.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++-- src/H5Opkg.h | 1 + test/tattr.c | 6 ++- 8 files changed, 232 insertions(+), 78 deletions(-) diff --git a/bin/trace b/bin/trace index 241e6dd..445e5a2 100755 --- a/bin/trace +++ b/bin/trace @@ -228,7 +228,7 @@ sub rewrite_func ($$$$$) { } } } - $trace = "H5TRACE" . scalar(@arg_str) . "(\"$rettype\",\""; + $trace = "H5TRACE" . scalar(@arg_str) . "(\"$rettype\", \""; $trace .= join("", @arg_str) . "\""; my $len = 4 + length $trace; for (@arg_name) { @@ -236,7 +236,7 @@ sub rewrite_func ($$$$$) { $trace .= ",\n $_"; $len = 13 + length; } else { - $trace .= ",$_"; + $trace .= ", $_"; $len += 1 + length; } } diff --git a/src/H5A.c b/src/H5A.c index 05a029e..ca75372 100644 --- a/src/H5A.c +++ b/src/H5A.c @@ -73,7 +73,6 @@ static H5A_t *H5A_open_by_name(const H5G_loc_t *loc, const char *name, static herr_t H5A_write(H5A_t *attr, const H5T_t *mem_type, const void *buf, hid_t dxpl_id); static herr_t H5A_read(const H5A_t *attr, const H5T_t *mem_type, void *buf, hid_t dxpl_id); static hsize_t H5A_get_storage_size(const H5A_t *attr); -static herr_t H5A_find_idx_by_name(const void *mesg, unsigned idx, void *op_data); /*********************/ @@ -233,7 +232,6 @@ H5Acreate(hid_t loc_id, const char *name, hid_t type_id, hid_t space_id, hid_t ret_value; /* Return value */ FUNC_ENTER_API(H5Acreate, FAIL) - H5TRACE5("i","isiii",loc_id,name,type_id,space_id,plist_id); /* check arguments */ if(H5I_FILE == H5I_get_type(loc_id) || H5I_ATTR == H5I_get_type(loc_id)) @@ -288,14 +286,12 @@ done: */ static hid_t H5A_create(const H5G_loc_t *loc, const char *name, const H5T_t *type, - const H5S_t *space, hid_t acpl_id, hid_t dxpl_id) + const H5S_t *space, hid_t acpl_id, hid_t dxpl_id) { H5A_t *attr = NULL; - H5A_iter_cb1 cb; /* Iterator callback */ - H5P_genplist_t *ac_plist=NULL; /* New Property list */ H5O_shared_t sh_mesg; - htri_t tri_ret; /* htri_t return value */ - hid_t ret_value = FAIL; + htri_t tri_ret; /* htri_t return value */ + hid_t ret_value; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5A_create) @@ -308,13 +304,14 @@ H5A_create(const H5G_loc_t *loc, const char *name, const H5T_t *type, /* Reset shared message information */ HDmemset(&sh_mesg, 0, sizeof(H5O_shared_t)); - /* Iterate over the existing attributes to check for duplicates */ - cb.name = name; - cb.idx = (-1); -/* XXX: Add support/test for dense attribute storage */ - if((ret_value = H5O_msg_iterate(loc->oloc, H5O_ATTR_ID, H5A_find_idx_by_name, &cb, dxpl_id)) < 0) - HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, FAIL, "error iterating over attributes") - if(ret_value > 0) + /* Check for existing attribute with same name */ + /* (technically, the "attribute create" operation will fail for a duplicated + * name, but it's going to be hard to unwind all the special cases on + * failure, so just check first, for now - QAK) + */ + if((tri_ret = H5O_attr_exists(loc->oloc, name, H5AC_ind_dxpl_id)) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, FAIL, "error checking attributes") + else if(tri_ret > 0) HGOTO_ERROR(H5E_ATTR, H5E_ALREADYEXISTS, FAIL, "attribute already exists") /* Check if the dataspace has an extent set (or is NULL) */ @@ -329,8 +326,10 @@ H5A_create(const H5G_loc_t *loc, const char *name, const H5T_t *type, if(acpl_id == H5P_DEFAULT) attr->encoding = H5F_DEFAULT_CSET; else { + H5P_genplist_t *ac_plist; /* ACPL Property list */ + /* Get a local copy of the attribute creation property list */ - if (NULL == (ac_plist = H5I_object(acpl_id))) + if(NULL == (ac_plist = H5I_object(acpl_id))) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a property list") if(H5P_get(ac_plist, H5P_STRCRT_CHAR_ENCODING_NAME, &(attr->encoding)) < 0) @@ -420,7 +419,7 @@ H5A_create(const H5G_loc_t *loc, const char *name, const H5T_t *type, HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open") attr->obj_opened = TRUE; - /* Create the attribute message */ + /* Create the attribute on the object */ if(H5O_attr_create(&(attr->oloc), dxpl_id, attr) < 0) HGOTO_ERROR(H5E_ATTR, H5E_CANTINSERT, FAIL, "unable to create attribute in object header") @@ -442,50 +441,6 @@ done: /*-------------------------------------------------------------------------- NAME - H5A_find_idx_by_name - PURPOSE - Iterator callback to determine the index of a attribute - USAGE - herr_t H5A_find_idx_by_name (mesg, idx, op_data) - const H5A_t *mesg; IN: Pointer to attribute - unsigned idx; IN: Index of attribute - void *op_data; IN: Op data passed in - RETURNS - Non-negative on success, negative on failure - - DESCRIPTION - This function determines if an attribute matches the name to search - for (from the 'op_data') and sets the index value in the 'op_data'. ---------------------------------------------------------------------------*/ -static herr_t -H5A_find_idx_by_name(const void *_mesg, unsigned idx, void *_op_data) -{ - const H5A_t *mesg = (const H5A_t *)_mesg; /* Pointer to attribute */ - H5A_iter_cb1 *op_data = (H5A_iter_cb1 *)_op_data; /* Pointer to op data */ - int ret_value; /* Return value */ - - FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5A_find_idx_by_name) - - HDassert(mesg); - HDassert(op_data); - - /* - * Compare found attribute name to queried name and set the idx in the - * callback info if names are the same. - */ - if(HDstrcmp(mesg->name, op_data->name) == 0) { - op_data->idx = idx; - ret_value = H5_ITER_STOP; - } /* end if */ - else - ret_value = H5_ITER_CONT; - - FUNC_LEAVE_NOAPI(ret_value) -} /* H5A_find_idx_by_name() */ - - -/*-------------------------------------------------------------------------- - NAME H5Aopen_name PURPOSE Opens an attribute for an object by looking up the attribute name diff --git a/src/H5Adense.c b/src/H5Adense.c index 9d98dc0..7395189 100644 --- a/src/H5Adense.c +++ b/src/H5Adense.c @@ -183,7 +183,7 @@ H5FL_BLK_DEFINE(ser_attr); *------------------------------------------------------------------------- */ static herr_t -H5A_dense_build_table_cb(const H5A_t *attr, uint8_t mesg_flags, void *_udata) +H5A_dense_build_table_cb(const H5A_t *attr, unsigned mesg_flags, void *_udata) { H5A_dense_bt_ud_t *udata = (H5A_dense_bt_ud_t *)_udata; /* 'User data' passed in */ herr_t ret_value = H5_ITER_CONT; /* Return value */ @@ -1172,6 +1172,69 @@ done: /*------------------------------------------------------------------------- + * Function: H5A_dense_exists + * + * Purpose: Check if an attribute exists in dense storage structures for + * an object + * + * Return: Non-negative on success/Negative on failure + * + * Programmer: Quincey Koziol + * koziol@hdfgroup.org + * Dec 11 2006 + * + *------------------------------------------------------------------------- + */ +htri_t +H5A_dense_exists(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, const char *name) +{ + H5A_bt2_ud_common_t udata; /* User data for v2 B-tree modify */ + H5HF_t *fheap = NULL; /* Fractal heap handle */ + htri_t ret_value = TRUE; /* Return value */ + + FUNC_ENTER_NOAPI(H5A_dense_exists, NULL) + + /* + * Check arguments. + */ + HDassert(f); + HDassert(oh); + HDassert(name); + + /* Open the fractal heap */ + if(NULL == (fheap = H5HF_open(f, dxpl_id, oh->attr_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + + /* Create the "udata" information for v2 B-tree record 'find' */ + udata.f = f; + udata.dxpl_id = dxpl_id; + udata.fheap = fheap; + udata.name = name; + udata.name_hash = H5_checksum_lookup3(name, HDstrlen(name), 0); + udata.flags = 0; + udata.corder = -1; /* XXX: None yet */ + udata.found_op = NULL; /* v2 B-tree comparison callback */ + udata.found_op_data = NULL; + +/* XXX: test for shared attributes */ + /* Find the attribute in the 'name' index */ + if(H5B2_find(f, dxpl_id, H5A_BT2_NAME, oh->name_bt2_addr, &udata, NULL, NULL) < 0) { + /* Assume that the failure was just not finding the attribute & clear stack */ + H5E_clear_stack(NULL); + + ret_value = FALSE; + } /* end if */ + +done: + /* Release resources */ + if(fheap && H5HF_close(fheap, dxpl_id) < 0) + HDONE_ERROR(H5E_ATTR, H5E_CLOSEERROR, FAIL, "can't close fractal heap") + + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5A_dense_exists() */ + + +/*------------------------------------------------------------------------- * Function: H5A_dense_delete * * Purpose: Delete all dense storage structures for attributes on an object diff --git a/src/H5Apkg.h b/src/H5Apkg.h index 85be22e..11dd43d 100644 --- a/src/H5Apkg.h +++ b/src/H5Apkg.h @@ -122,7 +122,7 @@ typedef struct { } H5A_attr_table_t; /* Attribute iteration operator for internal library callbacks */ -typedef herr_t (*H5A_lib_iterate_t)(const H5A_t *attr, uint8_t mesg_flags, +typedef herr_t (*H5A_lib_iterate_t)(const H5A_t *attr, unsigned mesg_flags, void *op_data); /* Describe kind of callback to make for each attribute */ @@ -172,6 +172,8 @@ H5_DLL herr_t H5A_dense_iterate(H5F_t *f, hid_t dxpl_id, hid_t loc_id, unsigned *last_attr, const H5A_attr_iterate_t *attr_op, void *op_data); H5_DLL herr_t H5A_dense_remove(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, const char *name); +H5_DLL htri_t H5A_dense_exists(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, + const char *name); /* Attribute table operations */ H5_DLL herr_t H5A_dense_build_table(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, diff --git a/src/H5B2.c b/src/H5B2.c index 358e8ac..d88c63c 100644 --- a/src/H5B2.c +++ b/src/H5B2.c @@ -328,10 +328,10 @@ herr_t H5B2_find(H5F_t *f, hid_t dxpl_id, const H5B2_class_t *type, haddr_t addr, void *udata, H5B2_found_t op, void *op_data) { - H5B2_t *bt2=NULL; /* Pointer to the B-tree header */ - H5RC_t *bt2_shared=NULL; /* Pointer to ref-counter for shared B-tree info */ + H5B2_t *bt2 = NULL; /* Pointer to the B-tree header */ + H5RC_t *bt2_shared = NULL; /* Pointer to ref-counter for shared B-tree info */ H5B2_shared_t *shared; /* Pointer to B-tree's shared information */ - hbool_t incr_rc=FALSE; /* Flag to indicate that we've incremented the B-tree's shared info reference count */ + hbool_t incr_rc = FALSE; /* Flag to indicate that we've incremented the B-tree's shared info reference count */ H5B2_node_ptr_t curr_node_ptr; /* Node pointer info for current node */ unsigned depth; /* Current depth of the tree */ int cmp; /* Comparison value of records */ @@ -346,16 +346,16 @@ H5B2_find(H5F_t *f, hid_t dxpl_id, const H5B2_class_t *type, haddr_t addr, HDassert(H5F_addr_defined(addr)); /* Look up the B-tree header */ - if (NULL == (bt2 = H5AC_protect(f, dxpl_id, H5AC_BT2_HDR, addr, type, NULL, H5AC_READ))) + if(NULL == (bt2 = H5AC_protect(f, dxpl_id, H5AC_BT2_HDR, addr, type, NULL, H5AC_READ))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "unable to load B-tree header") /* Safely grab pointer to reference counted shared B-tree info, so we can release the B-tree header if necessary */ - bt2_shared=bt2->shared; + bt2_shared = bt2->shared; H5RC_INC(bt2_shared); - incr_rc=TRUE; + incr_rc = TRUE; /* Get the pointer to the shared B-tree info */ - shared=H5RC_GET_OBJ(bt2_shared); + shared = H5RC_GET_OBJ(bt2_shared); HDassert(shared); /* Make copy of the root node pointer to start search with */ @@ -370,7 +370,7 @@ H5B2_find(H5F_t *f, hid_t dxpl_id, const H5B2_class_t *type, haddr_t addr, bt2 = NULL; /* Check for empty tree */ - if(curr_node_ptr.node_nrec==0) + if(curr_node_ptr.node_nrec == 0) HGOTO_ERROR(H5E_BTREE, H5E_NOTFOUND, FAIL, "B-tree has no records") /* Walk down B-tree to find record or leaf node where record is located */ diff --git a/src/H5Oattribute.c b/src/H5Oattribute.c index 7d22319..7d5e496 100644 --- a/src/H5Oattribute.c +++ b/src/H5Oattribute.c @@ -143,7 +143,7 @@ typedef struct { /*------------------------------------------------------------------------- - * Function: H5O_msg_attr_to_dense_cb + * Function: H5O_attr_to_dense_cb * * Purpose: Object header iterator callback routine to convert compact * attributes to dense attributes @@ -157,13 +157,13 @@ typedef struct { *------------------------------------------------------------------------- */ static herr_t -H5A_attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg/*in,out*/, +H5O_attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg/*in,out*/, unsigned UNUSED sequence, unsigned *oh_flags_ptr, void *_udata/*in,out*/) { H5O_iter_cvt_t *udata = (H5O_iter_cvt_t *)_udata; /* Operator user data */ herr_t ret_value = H5_ITER_CONT; /* Return value */ - FUNC_ENTER_NOAPI_NOINIT(H5A_attr_to_dense_cb) + FUNC_ENTER_NOAPI_NOINIT(H5O_attr_to_dense_cb) /* check args */ HDassert(oh); @@ -182,7 +182,7 @@ H5A_attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg/*in,out*/, done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5A_attr_to_dense_cb() */ +} /* end H5O_attr_to_dense_cb() */ /*------------------------------------------------------------------------- @@ -248,7 +248,7 @@ HDfprintf(stderr, "%s: converting attributes to dense storage\n", FUNC); /* Iterate over existing attributes, moving them to dense storage */ /* XXX: Test this with shared attributes */ - op.lib_op = H5A_attr_to_dense_cb; + op.lib_op = H5O_attr_to_dense_cb; if(H5O_msg_iterate_real(loc->file, oh, H5O_MSG_ATTR, TRUE, op, &udata, dxpl_id, &oh_flags) < 0) HGOTO_ERROR(H5E_ATTR, H5E_CANTCONVERT, FAIL, "error converting attributes to dense storage") } /* end if */ @@ -936,6 +936,7 @@ H5O_attr_remove_cb(H5O_t *oh, H5O_mesg_t *mesg/*in,out*/, /* check args */ HDassert(oh); HDassert(mesg); + HDassert(!udata->found); /* Check for shared message */ if(mesg->flags & H5O_MSG_FLAG_SHARED) { @@ -1151,3 +1152,131 @@ done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5O_attr_count */ + +/*------------------------------------------------------------------------- + * Function: H5O_attr_exists_cb + * + * Purpose: Object header iterator callback routine to check for an + * attribute stored compactly, by name. + * + * Return: Non-negative on success/Negative on failure + * + * Programmer: Quincey Koziol + * koziol@hdfgroup.org + * Dec 11 2006 + * + *------------------------------------------------------------------------- + */ +static herr_t +H5O_attr_exists_cb(H5O_t *oh, H5O_mesg_t *mesg/*in,out*/, + unsigned UNUSED sequence, unsigned UNUSED *oh_flags_ptr, void *_udata/*in,out*/) +{ + H5O_iter_rm_t *udata = (H5O_iter_rm_t *)_udata; /* Operator user data */ + herr_t ret_value = H5_ITER_CONT; /* Return value */ + + FUNC_ENTER_NOAPI_NOINIT(H5O_attr_exists_cb) + + /* check args */ + HDassert(oh); + HDassert(mesg); + HDassert(!udata->found); + + /* Check for shared message */ + if(mesg->flags & H5O_MSG_FLAG_SHARED) { + H5A_t shared_attr; /* Copy of shared attribute */ + + /* + * If the message is shared then then the native pointer points to an + * H5O_MSG_SHARED message. We use that information to look up the real + * message in the global heap or some other object header. + */ + if(NULL == H5O_shared_read(udata->f, udata->dxpl_id, mesg->native, H5O_MSG_ATTR, &shared_attr)) + HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, H5_ITER_ERROR, "unable to read shared attribute") + + /* Check for correct attribute message */ + if(HDstrcmp(shared_attr.name, udata->name) == 0) + /* Indicate that this message is the attribute sought */ + udata->found = TRUE; + + /* Release copy of shared attribute */ + H5O_attr_reset(&shared_attr); + } /* end if */ + else { + /* Check for correct attribute message */ + if(HDstrcmp(((H5A_t *)mesg->native)->name, udata->name) == 0) + /* Indicate that this message is the attribute sought */ + udata->found = TRUE; + } /* end else */ + + /* Check for finding correct message to delete */ + if(udata->found) { + /* Stop iterating */ + ret_value = H5_ITER_STOP; + } /* end if */ + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5O_attr_exists_cb() */ + + +/*------------------------------------------------------------------------- + * Function: H5O_attr_exists + * + * Purpose: Determine if an attribute with a particular name exists on an object + * + * Return: Non-negative on success/Negative on failure + * + * Programmer: Quincey Koziol + * Monday, December 11, 2006 + * + *------------------------------------------------------------------------- + */ +htri_t +H5O_attr_exists(const H5O_loc_t *loc, const char *name, hid_t dxpl_id) +{ + H5O_t *oh = NULL; /* Pointer to actual object header */ + unsigned oh_flags = H5AC__NO_FLAGS_SET; /* Metadata cache flags for object header */ + htri_t ret_value; /* Return value */ + + FUNC_ENTER_NOAPI_NOINIT(H5O_attr_exists) + + /* Check arguments */ + HDassert(loc); + HDassert(name); + + /* Protect the object header to iterate over */ + if(NULL == (oh = H5AC_protect(loc->file, dxpl_id, H5AC_OHDR, loc->addr, NULL, NULL, H5AC_READ))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTLOAD, FAIL, "unable to load object header") + + /* Check for attributes stored densely */ + if(H5F_addr_defined(oh->attr_fheap_addr)) { + /* Check if attribute exists in dense storage */ + if((ret_value = H5A_dense_exists(loc->file, dxpl_id, oh, name)) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_BADITER, FAIL, "error checking for existence of attribute") + } /* end if */ + else { + H5O_iter_rm_t udata; /* User data for callback */ + H5O_mesg_operator_t op; /* Wrapper for operator */ + + /* Set up user data for callback */ + udata.f = loc->file; + udata.dxpl_id = dxpl_id; + udata.name = name; + udata.found = FALSE; + + /* Iterate over existing attributes, checking for attribute with same name */ + op.lib_op = H5O_attr_exists_cb; + if(H5O_msg_iterate_real(loc->file, oh, H5O_MSG_ATTR, TRUE, op, &udata, dxpl_id, &oh_flags) < 0) + HGOTO_ERROR(H5E_ATTR, H5E_BADITER, FAIL, "error checking for existence of attribute") + + /* Check that we found the attribute */ + ret_value = udata.found; + } /* end else */ + +done: + if(oh && H5AC_unprotect(loc->file, dxpl_id, H5AC_OHDR, loc->addr, oh, H5AC__NO_FLAGS_SET) < 0) + HDONE_ERROR(H5E_ATTR, H5E_PROTECT, FAIL, "unable to release object header") + + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5O_attr_exists */ + diff --git a/src/H5Opkg.h b/src/H5Opkg.h index 07ccdd4..e0c7292 100644 --- a/src/H5Opkg.h +++ b/src/H5Opkg.h @@ -432,6 +432,7 @@ H5_DLL herr_t H5O_attr_iterate(hid_t loc_id, const H5O_loc_t *loc, hid_t dxpl_id H5_DLL herr_t H5O_attr_remove(const H5O_loc_t *loc, const char *name, hid_t dxpl_id); H5_DLL int H5O_attr_count(const H5O_loc_t *loc, hid_t dxpl_id); +H5_DLL htri_t H5O_attr_exists(const H5O_loc_t *loc, const char *name, hid_t dxpl_id); /* These functions operate on object locations */ H5_DLL H5O_loc_t *H5O_get_loc(hid_t id); diff --git a/test/tattr.c b/test/tattr.c index 1d1a8da..12eebec 100644 --- a/test/tattr.c +++ b/test/tattr.c @@ -1770,6 +1770,10 @@ HDfprintf(stderr, "max_compact = %u, min_dense = %u\n", max_compact, min_dense); ret = H5Aclose(attr); CHECK(ret, FAIL, "H5Aclose"); + /* Attempt to add attribute again, which should fail */ + attr = H5Acreate(dataset, attrname, H5T_NATIVE_UINT, sid, H5P_DEFAULT); + VERIFY(attr, FAIL, "H5Acreate"); + /* Close dataspace */ ret = H5Sclose(sid); CHECK(ret, FAIL, "H5Sclose"); @@ -1955,7 +1959,7 @@ HDfprintf(stderr, "max_compact = %u, min_dense = %u\n", max_compact, min_dense); is_dense = H5O_is_attr_dense_test(dataset); VERIFY(is_dense, FALSE, "H5O_is_attr_dense_test"); - /* Add attributes, until just before coverting to dense storage */ + /* Add attributes, until well into dense storage */ for(u = 0; u < (max_compact * 2); u++) { /* Create attribute */ sprintf(attrname, "attr %02u", u); -- cgit v0.12