From bd346629483722e7bae94521a33df4ec7a58bf0b Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Thu, 4 Sep 2003 16:43:37 -0500 Subject: [svn-r7445] Purpose: Fix, of a sort Description: Some of the code would get an object from the cache via the H5AC_find() function and then modify the returned object. This behavior is incorrect as the pointer returned via the H5AC_find() function is supposed to be read only. Solution: Changed the H5AC_finds to H5AC_protect() instead and added the appropriate H5AC_unprotect() function. Platforms tested: (simulated h5committest by hand since it doesn't work for me) Linux (Fortran, C++) Solaris (Fortran) AIX (Fortran, C++) SGI (Parallel, Fortran) --- src/H5B.c | 38 +++++++++++++++++++++++++++++++------- src/H5Gnode.c | 7 +++++-- src/H5HG.c | 18 ++++++++++++------ src/H5HL.c | 36 ++++++++++++++++++++++++++---------- src/H5O.c | 5 ++++- 5 files changed, 78 insertions(+), 26 deletions(-) diff --git a/src/H5B.c b/src/H5B.c index ca9fb47..41aafce 100644 --- a/src/H5B.c +++ b/src/H5B.c @@ -821,11 +821,16 @@ H5B_split(H5F_t *f, hid_t dxpl_id, const H5B_class_t *type, H5B_t *old_bt, haddr new_bt->right = old_bt->right; if (H5F_addr_defined(old_bt->right)) { - if (NULL == (tmp_bt = H5AC_find(f, dxpl_id, H5AC_BT, old_bt->right, type, udata))) + if (NULL == (tmp_bt = H5AC_protect(f, dxpl_id, H5AC_BT, old_bt->right, type, udata))) HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "unable to load right sibling") + tmp_bt->cache_info.dirty = TRUE; tmp_bt->left = *new_addr_p; + + if (H5AC_unprotect(f, dxpl_id, H5AC_BT, old_bt->right, tmp_bt, FALSE) != SUCCEED) + HDONE_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release B-tree node") } + old_bt->right = *new_addr_p; done: @@ -1001,23 +1006,30 @@ H5B_insert(H5F_t *f, hid_t dxpl_id, const H5B_class_t *type, haddr_t addr, HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate file space to move root") /* update the new child's left pointer */ - if (NULL == (bt = H5AC_find(f, dxpl_id, H5AC_BT, child, type, udata))) + if (NULL == (bt = H5AC_protect(f, dxpl_id, H5AC_BT, child, type, udata))) HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "unable to load new child") + bt->cache_info.dirty = TRUE; bt->left = old_root; + if (H5AC_unprotect(f, dxpl_id, H5AC_BT, child, bt, FALSE) != SUCCEED) + HDONE_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release new child") + /* * Move the node to the new location by checking it out & checking it in * at the new location -QAK */ /* Bring the old root into the cache if it's not already */ - if (NULL == (bt = H5AC_find(f, dxpl_id, H5AC_BT, addr, type, udata))) + if (NULL == (bt = H5AC_protect(f, dxpl_id, H5AC_BT, addr, type, udata))) HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "unable to load new child") /* Make certain the old root info is marked as dirty before moving it, */ /* so it is certain to be written out at the new location */ bt->cache_info.dirty = TRUE; + if (H5AC_unprotect(f, dxpl_id, H5AC_BT, addr, bt, FALSE) != SUCCEED) + HDONE_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release new child") + /* Make a copy of the old root information */ if (NULL == (bt = H5B_copy(f, bt))) HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "unable to copy old root") @@ -1749,16 +1761,24 @@ H5B_remove_helper(H5F_t *f, hid_t dxpl_id, haddr_t addr, const H5B_class_t *type bt->ndirty = 0; if (level>0) { if (H5F_addr_defined(bt->left)) { - if (NULL==(sibling=H5AC_find(f, dxpl_id, H5AC_BT, bt->left, type, udata))) - HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, H5B_INS_ERROR, "unable to unlink node from tree") + if (NULL == (sibling = H5AC_protect(f, dxpl_id, H5AC_BT, bt->left, type, udata))) + HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, H5B_INS_ERROR, "unable to load node from tree") + sibling->right = bt->right; sibling->cache_info.dirty = TRUE; + + if (H5AC_unprotect(f, dxpl_id, H5AC_BT, bt->left, sibling, FALSE) != SUCCEED) + HDONE_ERROR(H5E_BTREE, H5E_PROTECT, H5B_INS_ERROR, "unable to release node from tree") } if (H5F_addr_defined(bt->right)) { - if (NULL==(sibling=H5AC_find(f, dxpl_id, H5AC_BT, bt->right, type, udata))) + if (NULL == (sibling = H5AC_protect(f, dxpl_id, H5AC_BT, bt->right, type, udata))) HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, H5B_INS_ERROR, "unable to unlink node from tree") + sibling->left = bt->left; sibling->cache_info.dirty = TRUE; + + if (H5AC_unprotect(f, dxpl_id, H5AC_BT, bt->right, sibling, FALSE) != SUCCEED) + HDONE_ERROR(H5E_BTREE, H5E_PROTECT, H5B_INS_ERROR, "unable to release node from tree") } bt->left = HADDR_UNDEF; bt->right = HADDR_UNDEF; @@ -1912,13 +1932,17 @@ H5B_remove(H5F_t *f, hid_t dxpl_id, const H5B_class_t *type, haddr_t addr, void * If the B-tree is now empty then make sure we mark the root node as * being at level zero */ - if (NULL==(bt=H5AC_find(f, dxpl_id, H5AC_BT, addr, type, udata))) + if (NULL == (bt = H5AC_protect(f, dxpl_id, H5AC_BT, addr, type, udata))) HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "unable to load B-tree root node") + if (0==bt->nchildren && 0!=bt->level) { bt->level = 0; bt->cache_info.dirty = TRUE; } + if (H5AC_unprotect(f, dxpl_id, H5AC_BT, addr, bt, FALSE) != SUCCEED) + HGOTO_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release node") + #ifdef H5B_DEBUG H5B_assert(f, dxpl_id, addr, type, udata); #endif diff --git a/src/H5Gnode.c b/src/H5Gnode.c index 3524159..d70a5c0 100644 --- a/src/H5Gnode.c +++ b/src/H5Gnode.c @@ -977,8 +977,10 @@ H5G_node_insert(H5F_t *f, hid_t dxpl_id, haddr_t addr, void UNUSED *_lt_key, if (H5G_node_create(f, dxpl_id, H5B_INS_FIRST, NULL, NULL, NULL, new_node_p/*out*/)<0) HGOTO_ERROR(H5E_SYM, H5E_CANTINIT, H5B_INS_ERROR, "unable to split symbol table node"); - if (NULL==(snrt=H5AC_find(f, dxpl_id, H5AC_SNODE, *new_node_p, NULL, NULL))) + + if (NULL == (snrt = H5AC_protect(f, dxpl_id, H5AC_SNODE, *new_node_p, NULL, NULL))) HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, H5B_INS_ERROR, "unable to split symbol table node"); + HDmemcpy(snrt->entry, sn->entry + H5F_SYM_LEAF_K(f), H5F_SYM_LEAF_K(f) * sizeof(H5G_entry_t)); snrt->nsyms = H5F_SYM_LEAF_K(f); @@ -1007,6 +1009,8 @@ H5G_node_insert(H5F_t *f, hid_t dxpl_id, haddr_t addr, void UNUSED *_lt_key, } } + if (snrt && H5AC_unprotect(f, dxpl_id, H5AC_SNODE, *new_node_p, snrt, FALSE) != SUCCEED) + HDONE_ERROR(H5E_SYM, H5E_PROTECT, H5B_INS_ERROR, "unable to release symbol table node"); } else { /* Where to insert the new entry? */ ret_value = H5B_INS_NOOP; @@ -1285,7 +1289,6 @@ H5G_node_iterate (H5F_t *f, hid_t dxpl_id, void UNUSED *_lt_key, haddr_t addr, HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, H5B_ITER_ERROR, "memory allocation failed"); for (i=0; ientry[i].name_off; - sn = NULL; /* * Iterate over the symbol table node entries. diff --git a/src/H5HG.c b/src/H5HG.c index f349299..559bc1c 100644 --- a/src/H5HG.c +++ b/src/H5HG.c @@ -897,8 +897,9 @@ H5HG_link (H5F_t *f, hid_t dxpl_id, H5HG_t *hobj, int adjust) HGOTO_ERROR (H5E_HEAP, H5E_WRITEERROR, FAIL, "no write intent on file"); /* Load the heap */ - if (NULL==(heap=H5AC_find(f, dxpl_id, H5AC_GHEAP, hobj->addr, NULL, NULL))) - HGOTO_ERROR (H5E_HEAP, H5E_CANTLOAD, FAIL, "unable to load heap"); + if (NULL == (heap = H5AC_protect(f, dxpl_id, H5AC_GHEAP, hobj->addr, NULL, NULL))) + HGOTO_ERROR(H5E_HEAP, H5E_CANTLOAD, FAIL, "unable to load heap"); + assert (hobj->idx>0 && hobj->idxnalloc); assert (heap->obj[hobj->idx].begin); if (heap->obj[hobj->idx].nrefs+adjust<0) @@ -913,6 +914,9 @@ H5HG_link (H5F_t *f, hid_t dxpl_id, H5HG_t *hobj, int adjust) ret_value=heap->obj[hobj->idx].nrefs; done: + if (heap && H5AC_unprotect(f, dxpl_id, H5AC_GHEAP, hobj->addr, heap, FALSE) != SUCCEED && ret_value != FAIL) + HDONE_ERROR(H5E_HEAP, H5E_PROTECT, FAIL, "unable to release object header"); + FUNC_LEAVE_NOAPI(ret_value); } @@ -950,8 +954,9 @@ H5HG_remove (H5F_t *f, hid_t dxpl_id, H5HG_t *hobj) HGOTO_ERROR (H5E_HEAP, H5E_WRITEERROR, FAIL, "no write intent on file"); /* Load the heap */ - if (NULL==(heap=H5AC_find(f, dxpl_id, H5AC_GHEAP, hobj->addr, NULL, NULL))) - HGOTO_ERROR (H5E_HEAP, H5E_CANTLOAD, FAIL, "unable to load heap"); + if (NULL == (heap = H5AC_protect(f, dxpl_id, H5AC_GHEAP, hobj->addr, NULL, NULL))) + HGOTO_ERROR(H5E_HEAP, H5E_CANTLOAD, FAIL, "unable to load heap"); + assert (hobj->idx>0 && hobj->idxnalloc); assert (heap->obj[hobj->idx].begin); obj_start = heap->obj[hobj->idx].begin; @@ -991,7 +996,6 @@ H5HG_remove (H5F_t *f, hid_t dxpl_id, H5HG_t *hobj) H5_CHECK_OVERFLOW(heap->size,size_t,hsize_t); H5MF_xfree(f, H5FD_MEM_GHEAP, dxpl_id, heap->addr, (hsize_t)heap->size); H5AC_flush (f, dxpl_id, H5AC_GHEAP, heap->addr, H5F_FLUSH_INVALIDATE); - heap = NULL; } else { /* * If the heap is in the CWFS list then advance it one position. The @@ -1013,7 +1017,9 @@ H5HG_remove (H5F_t *f, hid_t dxpl_id, H5HG_t *hobj) } } + if (H5AC_unprotect(f, dxpl_id, H5AC_GHEAP, hobj->addr, heap, FALSE) != SUCCEED) + HDONE_ERROR(H5E_HEAP, H5E_PROTECT, FAIL, "unable to release object header"); + done: FUNC_LEAVE_NOAPI(ret_value); } - diff --git a/src/H5HL.c b/src/H5HL.c index 126872a..81c5d88 100644 --- a/src/H5HL.c +++ b/src/H5HL.c @@ -768,12 +768,14 @@ H5HL_insert(H5F_t *f, hid_t dxpl_id, haddr_t addr, size_t buf_size, const void * assert(H5F_addr_defined(addr)); assert(buf_size > 0); assert(buf); + if (0==(f->intent & H5F_ACC_RDWR)) HGOTO_ERROR (H5E_HEAP, H5E_WRITEERROR, (size_t)(-1), "no write intent on file"); - if (NULL == (heap = H5AC_find(f, dxpl_id, H5AC_LHEAP, addr, NULL, NULL))) - HGOTO_ERROR(H5E_HEAP, H5E_CANTLOAD, (size_t)(-1), "unable to load heap"); - heap->cache_info.dirty += 1; + if (NULL == (heap = H5AC_protect(f, dxpl_id, H5AC_LHEAP, addr, NULL, NULL))) + HGOTO_ERROR(H5E_HEAP, H5E_PROTECT, (size_t)(-1), "unable to load heap"); + + ++heap->cache_info.dirty; /* Cache this for later */ sizeof_hdr= H5HL_SIZEOF_HDR(f); @@ -893,6 +895,9 @@ H5HL_insert(H5F_t *f, hid_t dxpl_id, haddr_t addr, size_t buf_size, const void * ret_value=offset; done: + if (heap && H5AC_unprotect(f, dxpl_id, H5AC_LHEAP, addr, heap, FALSE) != SUCCEED && ret_value != (size_t)(-1)) + HDONE_ERROR(H5E_HEAP, H5E_PROTECT, (size_t)(-1), "unable to release object header"); + FUNC_LEAVE_NOAPI(ret_value); } @@ -933,18 +938,23 @@ H5HL_write(H5F_t *f, hid_t dxpl_id, haddr_t addr, size_t offset, size_t size, co assert(H5F_addr_defined(addr)); assert(buf); assert (offset==H5HL_ALIGN (offset)); + if (0==(f->intent & H5F_ACC_RDWR)) HGOTO_ERROR (H5E_HEAP, H5E_WRITEERROR, FAIL, "no write intent on file"); - if (NULL == (heap = H5AC_find(f, dxpl_id, H5AC_LHEAP, addr, NULL, NULL))) - HGOTO_ERROR(H5E_HEAP, H5E_CANTLOAD, FAIL, "unable to load heap"); + if (NULL == (heap = H5AC_protect(f, dxpl_id, H5AC_LHEAP, addr, NULL, NULL))) + HGOTO_ERROR(H5E_HEAP, H5E_PROTECT, FAIL, "unable to load heap"); + assert(offset < heap->mem_alloc); assert(offset + size <= heap->mem_alloc); - heap->cache_info.dirty += 1; + ++heap->cache_info.dirty; HDmemcpy(heap->chunk + H5HL_SIZEOF_HDR(f) + offset, buf, size); done: + if (heap && H5AC_unprotect(f, dxpl_id, H5AC_LHEAP, addr, heap, FALSE) != SUCCEED && ret_value != FAIL) + HDONE_ERROR(H5E_HEAP, H5E_PROTECT, FAIL, "unable to release object header"); + FUNC_LEAVE_NOAPI(ret_value); } #endif /* NOT_YET */ @@ -991,17 +1001,20 @@ H5HL_remove(H5F_t *f, hid_t dxpl_id, haddr_t addr, size_t offset, size_t size) assert(H5F_addr_defined(addr)); assert(size > 0); assert (offset==H5HL_ALIGN (offset)); + if (0==(f->intent & H5F_ACC_RDWR)) HGOTO_ERROR (H5E_HEAP, H5E_WRITEERROR, FAIL, "no write intent on file"); size = H5HL_ALIGN (size); - if (NULL == (heap = H5AC_find(f, dxpl_id, H5AC_LHEAP, addr, NULL, NULL))) - HGOTO_ERROR(H5E_HEAP, H5E_CANTLOAD, FAIL, "unable to load heap"); + + if (NULL == (heap = H5AC_protect(f, dxpl_id, H5AC_LHEAP, addr, NULL, NULL))) + HGOTO_ERROR(H5E_HEAP, H5E_PROTECT, FAIL, "unable to load heap"); + assert(offset < heap->mem_alloc); assert(offset + size <= heap->mem_alloc); - fl = heap->freelist; - heap->cache_info.dirty += 1; + fl = heap->freelist; + ++heap->cache_info.dirty; /* * Check if this chunk can be prepended or appended to an already @@ -1076,6 +1089,9 @@ H5HL_remove(H5F_t *f, hid_t dxpl_id, haddr_t addr, size_t offset, size_t size) heap->freelist = fl; done: + if (heap && H5AC_unprotect(f, dxpl_id, H5AC_LHEAP, addr, heap, FALSE) != SUCCEED && ret_value != FAIL) + HDONE_ERROR(H5E_HEAP, H5E_PROTECT, FAIL, "unable to release object header"); + FUNC_LEAVE_NOAPI(ret_value); } diff --git a/src/H5O.c b/src/H5O.c index 11d7f6c..17ca3a1 100644 --- a/src/H5O.c +++ b/src/H5O.c @@ -1648,7 +1648,7 @@ H5O_find_in_ohdr(H5F_t *f, hid_t dxpl_id, haddr_t addr, const H5O_class_t **type assert(type_p); /* Load the object header */ - if (NULL == (oh = H5AC_find(f, dxpl_id, H5AC_OHDR, addr, NULL, NULL))) + if (NULL == (oh = H5AC_protect(f, dxpl_id, H5AC_OHDR, addr, NULL, NULL))) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, UFAIL, "unable to load object header"); /* Scan through the messages looking for the right one */ @@ -1687,6 +1687,9 @@ H5O_find_in_ohdr(H5F_t *f, hid_t dxpl_id, haddr_t addr, const H5O_class_t **type ret_value=u; done: + if (oh && H5AC_unprotect(f, dxpl_id, H5AC_OHDR, addr, oh, FALSE) != SUCCEED && ret_value != UFAIL) + HDONE_ERROR(H5E_OHDR, H5E_PROTECT, UFAIL, "unable to release object header"); + FUNC_LEAVE_NOAPI(ret_value); } -- cgit v0.12