diff options
-rw-r--r-- | release_docs/RELEASE.txt | 2 | ||||
-rw-r--r-- | src/H5Oalloc.c | 183 | ||||
-rw-r--r-- | test/tattr.c | 84 |
3 files changed, 212 insertions, 57 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 9c4c47c..519608f 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -142,6 +142,8 @@ Bug Fixes since HDF5-1.8.0 release Library ------- + - Fixed a problem that could prevent the user from adding attributes (or + any object header message) in some circumstances. (NAF - 2009/02/12) - Fixed a bug that could cause problems when an attribute was added to a committed datatype using the committed datatype's datatype. (NAF - 2009/02/12) diff --git a/src/H5Oalloc.c b/src/H5Oalloc.c index 6d5d736..b2d38da 100644 --- a/src/H5Oalloc.c +++ b/src/H5Oalloc.c @@ -674,6 +674,7 @@ H5O_alloc_new_chunk(H5F_t *f, H5O_mesg_t *curr_msg; /* Pointer to current message to operate on */ size_t cont_size; /*continuation message size */ + size_t multi_size = 0; /* Size of all the messages in the last chunk */ int found_null = (-1); /* Best fit null message */ alloc_info found_attr = {-1, 0, 0, 0, 0}; /* Best fit attribute message */ alloc_info found_other = {-1, 0, 0, 0, 0}; /* Best fit other message */ @@ -769,11 +770,15 @@ H5O_alloc_new_chunk(H5F_t *f, found_other.null_msgno = null_msgno; } /* end if */ } /* end else */ - } /* end if */ + } else if(found_null < 0 && found_attr.msgno < 0 && found_other.msgno < 0 && msg_chunkno == oh->nchunks - 1) + /* Keep track of the total size of smaller messages in the last + * chunk, in case we need to move more than 1 message. + */ + multi_size += curr_msg->raw_size + H5O_SIZEOF_MSGHDR_OH(oh); } /* end else */ } /* end for */ - if(found_null < 0 && found_attr.msgno < 0 && found_other.msgno < 0) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, UFAIL, "unable to locate message to move") + if(found_null >= 0 || found_attr.msgno >= 0 || found_other.msgno >= 0) + multi_size = 0; /* * If we must move some other message to make room for the null @@ -782,14 +787,20 @@ H5O_alloc_new_chunk(H5F_t *f, * * Move other messages first, and attributes only as a last resort. * + * If all else fails, move every message in the last chunk. + * */ - if(found_null < 0) { - if(found_other.msgno < 0) - found_other = found_attr; + if(multi_size == 0) { + if(found_null < 0) { + if(found_other.msgno < 0) + found_other = found_attr; - HDassert(found_other.msgno >= 0); - size += H5O_SIZEOF_MSGHDR_OH(oh) + oh->mesg[found_other.msgno].raw_size; + HDassert(found_other.msgno >= 0); + size += H5O_SIZEOF_MSGHDR_OH(oh) + oh->mesg[found_other.msgno].raw_size; + } /* end if */ } /* end if */ + else + size += multi_size; /* * The total chunk size must include the requested space plus enough @@ -848,10 +859,58 @@ H5O_alloc_new_chunk(H5F_t *f, if(H5O_alloc_msgs(oh, (size_t)3) < 0) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, UFAIL, "can't allocate more space for messages") - /* Move message (that will be replaced with continuation message) - * to new chunk, if necessary. - */ - if(found_null < 0) { + if(multi_size > 0) { + /* Move all non-null messages in the last chunk to the new chunk. This + * should be extremely rare so we don't care too much about minimizing + * the space used */ + H5O_mesg_t *null_msg; /* Pointer to new null message */ + + /* Copy each message to the new location */ + for(u = 0, curr_msg = &oh->mesg[0]; u < oh->nmesgs; u++, curr_msg++) + if(curr_msg->chunkno == chunkno - 1) { + if(curr_msg->type->id == H5O_NULL_ID) { + /* Delete the null message */ + if(u < oh->nmesgs - 1) + HDmemmove(curr_msg, curr_msg + 1, ((oh->nmesgs - 1) - u) * sizeof(H5O_mesg_t)); + oh->nmesgs--; + } else { + /* Copy the raw data */ + HDmemcpy(p, curr_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh), + curr_msg->raw_size + H5O_SIZEOF_MSGHDR_OH(oh)); + + /* Update the message info */ + curr_msg->chunkno = chunkno; + curr_msg->raw = p + H5O_SIZEOF_MSGHDR_OH(oh); + + /* Account for copied message in new chunk */ + p += H5O_SIZEOF_MSGHDR_OH(oh) + curr_msg->raw_size; + size -= H5O_SIZEOF_MSGHDR_OH(oh) + curr_msg->raw_size; + } /* end else */ + } /* end if */ + + /* Create a null message spanning the entire last chunk */ + found_null = oh->nmesgs++; + null_msg = &(oh->mesg[found_null]); + null_msg->type = H5O_MSG_NULL; + null_msg->dirty = TRUE; + null_msg->native = NULL; + null_msg->raw = oh->chunk[chunkno-1].image + + ((chunkno == 1) ? H5O_SIZEOF_HDR(oh) : H5O_SIZEOF_CHKHDR_OH(oh)) + - H5O_SIZEOF_CHKSUM_OH(oh) + H5O_SIZEOF_MSGHDR_OH(oh); + null_msg->raw_size = oh->chunk[chunkno-1].size + - ((chunkno == 1) ? H5O_SIZEOF_HDR(oh) : H5O_SIZEOF_CHKHDR_OH(oh)) + - H5O_SIZEOF_MSGHDR_OH(oh); + null_msg->chunkno = chunkno - 1; + + HDassert(null_msg->raw_size >= cont_size); + + /* Remove any gap in the chunk */ + oh->chunk[chunkno-1].gap = 0; + + } else if(found_null < 0) { + /* Move message (that will be replaced with continuation message) + * to new chunk, if necessary. + */ H5O_mesg_t *null_msg; /* Pointer to new null message */ /* Create null message for space that message to copy currently occupies */ @@ -1788,9 +1847,10 @@ H5O_alloc_shrink_chunk(H5F_t *f, uint8_t *old_image = chunk->image; /* Old address of chunk's image in memory */ size_t old_size = chunk->size; /* Old size of chunk */ size_t new_size = chunk->size - chunk->gap; /* Size of shrunk chunk */ + size_t total_msg_size; /* Size of the messages in this chunk */ + size_t min_chunk_size = H5O_ALIGN_OH(oh, H5O_MIN_SIZE); /* Minimum chunk size */ size_t sizeof_chksum = H5O_SIZEOF_CHKSUM_OH(oh); /* Size of chunk checksum */ size_t sizeof_msghdr = H5O_SIZEOF_MSGHDR_OH(oh); /* Size of message header */ - size_t sizeof_chkhdr = H5O_SIZEOF_CHKHDR_OH(oh); /* Size of chunk header */ uint8_t new_size_flags = 0; /* New chunk #0 size flags */ hbool_t adjust_size_flags = FALSE; /* Whether to adjust the chunk #0 size flags */ size_t less_prfx_size = 0; /* Bytes removed from object header prefix */ @@ -1808,61 +1868,67 @@ H5O_alloc_shrink_chunk(H5F_t *f, */ for (u = oh->nmesgs - 1, curr_msg = &oh->mesg[u]; u < oh->nmesgs; u--, curr_msg--) { if ((H5O_NULL_ID == curr_msg->type->id) && (chunkno == curr_msg->chunkno)) { - /* Check if the message is the only one in the chunk */ - if (chunkno > 0 && (sizeof_msghdr + curr_msg->raw_size) == (new_size - sizeof_chkhdr)) { - /* Shrink the message to the minimum size */ - new_size -= curr_msg->raw_size; - curr_msg->raw_size = 0; - curr_msg->dirty = TRUE; - HDassert(new_size == sizeof_msghdr + sizeof_chkhdr); - - /* There are no more messages in this chunk, so we can exit the loop */ - break; - } /* end if */ - else { - /* Remove the message entirely */ - size_t shrink_size = curr_msg->raw_size + sizeof_msghdr; /* Amount to shrink the chunk by */ - /* If the current message is not at the end of the chunk, copy the - * data after it (except the checksum). - */ - if (curr_msg->raw + curr_msg->raw_size - < old_image + new_size - sizeof_chksum) { - unsigned v; /* Index */ - H5O_mesg_t *curr_msg2; - uint8_t *src = curr_msg->raw + curr_msg->raw_size; /* Source location */ - - /* Slide down the raw data */ - HDmemmove(curr_msg->raw - sizeof_msghdr, src, - (size_t)(old_image + new_size - sizeof_chksum - src)); - - /* Update the raw data pointers for messages after this one */ - for (v = 0, curr_msg2 = &oh->mesg[0]; v < oh->nmesgs; v++, curr_msg2++) - if ((chunkno == curr_msg2->chunkno) && (curr_msg2->raw > curr_msg->raw)) - curr_msg2->raw -= shrink_size; - } /* end if */ + size_t shrink_size = curr_msg->raw_size + sizeof_msghdr; /* Amount to shrink the chunk by */ + + /* If the current message is not at the end of the chunk, copy the + * data after it (except the checksum). + */ + if (curr_msg->raw + curr_msg->raw_size + < old_image + new_size - sizeof_chksum) { + unsigned v; /* Index */ + H5O_mesg_t *curr_msg2; + uint8_t *src = curr_msg->raw + curr_msg->raw_size; /* Source location */ + + /* Slide down the raw data */ + HDmemmove(curr_msg->raw - sizeof_msghdr, src, + (size_t)(old_image + new_size - sizeof_chksum - src)); + + /* Update the raw data pointers for messages after this one */ + for (v = 0, curr_msg2 = &oh->mesg[0]; v < oh->nmesgs; v++, curr_msg2++) + if ((chunkno == curr_msg2->chunkno) && (curr_msg2->raw > curr_msg->raw)) + curr_msg2->raw -= shrink_size; + } /* end if */ - /* Adjust the new chunk size */ - new_size -= shrink_size; + /* Adjust the new chunk size */ + new_size -= shrink_size; - /* Release any information/memory for the message */ - H5O_msg_free_mesg(curr_msg); + /* Release any information/memory for the message */ + H5O_msg_free_mesg(curr_msg); - /* Remove the deleted null message from list of messages */ - if (u < (oh->nmesgs - 1)) - HDmemmove(&oh->mesg[u], &oh->mesg[u+1], ((oh->nmesgs - 1) - u) * sizeof(H5O_mesg_t)); + /* Remove the deleted null message from list of messages */ + if (u < (oh->nmesgs - 1)) + HDmemmove(&oh->mesg[u], &oh->mesg[u+1], ((oh->nmesgs - 1) - u) * sizeof(H5O_mesg_t)); - /* Decrement # of messages */ - /* (Don't bother reducing size of message array for now) */ - oh->nmesgs--; - } /* end else */ + /* Decrement # of messages */ + /* (Don't bother reducing size of message array for now) */ + oh->nmesgs--; } /* end if */ } /* end for */ + /* Check if the chunk is too small, extend if necessary */ + total_msg_size = new_size - (chunkno == 0 ? H5O_SIZEOF_HDR(oh) : H5O_SIZEOF_CHKHDR_OH(oh)); + if(total_msg_size < min_chunk_size) { + HDassert(oh->alloc_nmesgs > oh->nmesgs); + oh->nmesgs++; + + /* Initialize new null message to make the chunk large enough */ + oh->mesg[oh->nmesgs].type = H5O_MSG_NULL; + oh->mesg[oh->nmesgs].dirty = TRUE; + oh->mesg[oh->nmesgs].native = NULL; + oh->mesg[oh->nmesgs].raw = old_image + new_size + sizeof_msghdr - sizeof_chksum; + oh->mesg[oh->nmesgs].raw_size = MAX(H5O_ALIGN_OH(oh, min_chunk_size - total_msg_size), + sizeof_msghdr) - sizeof_msghdr; + oh->mesg[oh->nmesgs].chunkno = chunkno; + + /* update the new chunk size */ + new_size += oh->mesg[oh->nmesgs].raw_size + sizeof_msghdr; + } + /* Check for changing the chunk #0 data size enough to need adjusting the flags */ if(oh->version > H5O_VERSION_1 && chunkno == 0) { uint64_t chunk0_newsize = new_size - H5O_SIZEOF_HDR(oh); /* New size of chunk 0's data */ size_t orig_prfx_size = 1 << (oh->flags & H5O_HDR_CHUNK0_SIZE); /* Original prefix size */ - + /* Check for moving to a 1-byte size encoding */ if (orig_prfx_size > 1 && chunk0_newsize <= 255) { less_prfx_size = orig_prfx_size - 1; @@ -1926,6 +1992,8 @@ H5O_alloc_shrink_chunk(H5F_t *f, } /* end if */ } /* end for */ + HDassert(new_size <= old_size); + /* Free the unused space in the file */ if (H5MF_xfree(f, H5FD_MEM_OHDR, dxpl_id, chunk->addr + new_size, (hsize_t)(old_size - new_size)) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTFREE, FAIL, "unable to shrink object header chunk") @@ -1933,3 +2001,4 @@ H5O_alloc_shrink_chunk(H5F_t *f, done: FUNC_LEAVE_NOAPI(ret_value) } /* H5O_alloc_shrink_chunk() */ + diff --git a/test/tattr.c b/test/tattr.c index b25d3a5..fe55023 100644 --- a/test/tattr.c +++ b/test/tattr.c @@ -9902,6 +9902,88 @@ test_attr_bug3(hid_t fcpl, hid_t fapl) /**************************************************************** ** +** test_attr_bug4(): Test basic H5A (attribute) code. +** Attempts to trigger a bug which would result in being +** unable to add an attribute to a named datatype. This +** happened when an object header chunk was too small to +** hold a continuation message and could not be extended. +** +****************************************************************/ +static void +test_attr_bug4(hid_t fcpl, hid_t fapl) +{ + hid_t fid; /* File ID */ + hid_t gid; /* Group ID */ + hid_t aid1, aid2, aid3; /* Attribute IDs */ + hid_t sid; /* Dataspace ID */ + hid_t tid; /* Datatype ID */ + hid_t did; /* Dataset ID */ + hsize_t dims[1] = {5}; /* Attribute dimensions */ + herr_t ret; /* Generic return status */ + + /* Output message about test being performed */ + MESSAGE(5, ("Testing that attributes can always be added to named datatypes\n")); + + /* Create dataspace */ + sid = H5Screate_simple(1, dims, NULL); + CHECK(sid, FAIL, "H5Screate_simple"); + + /* Create file */ + fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl, fapl); + CHECK(fid, FAIL, "H5Fcreate"); + + /* Open root group */ + gid = H5Gopen2(fid, "/", H5P_DEFAULT); + CHECK(gid, FAIL, "H5Gcreate2"); + + /* Create committed datatype */ + tid = H5Tcopy(H5T_STD_I32LE); + CHECK(tid, FAIL, "H5Tcopy"); + ret = H5Tcommit2(fid, "dtype", tid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Tcommit2"); + + /* Create dataset */ + did = H5Dcreate2(fid, "dset", tid, sid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + CHECK(did, FAIL, "H5Dcreate2"); + + /* Create attributes on group and dataset */ + aid1 = H5Acreate2(gid, "attr", tid, sid, H5P_DEFAULT, H5P_DEFAULT); + CHECK(aid1, FAIL, "H5Acreate2"); + aid2 = H5Acreate2(did, "attr", tid, sid, H5P_DEFAULT, H5P_DEFAULT); + CHECK(aid2, FAIL, "H5Acreate2"); + + /* Create attribute on datatype (this is the main test) */ + aid3 = H5Acreate2(tid, "attr", tid, sid, H5P_DEFAULT, H5P_DEFAULT); + CHECK(aid3, FAIL, "H5Acreate2"); + + /* Close IDs */ + ret = H5Aclose(aid3); + CHECK(ret, FAIL, "H5Aclose"); + + ret = H5Aclose(aid2); + CHECK(ret, FAIL, "H5Aclose"); + + ret = H5Aclose(aid1); + CHECK(ret, FAIL, "H5Aclose"); + + ret = H5Dclose(did); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Tclose(tid); + CHECK(ret, FAIL, "H5Tclose"); + + ret = H5Gclose(gid); + CHECK(ret, FAIL, "H5Gclose"); + + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + + ret = H5Sclose(sid); + CHECK(ret, FAIL, "H5Sclose"); +} /* test_attr_bug4() */ + +/**************************************************************** +** ** test_attr(): Main H5A (attribute) testing routine. ** ****************************************************************/ @@ -10043,6 +10125,7 @@ test_attr(void) test_attr_bug1(my_fcpl, my_fapl); /* Test odd allocation operations */ test_attr_bug2(my_fcpl, my_fapl); /* Test many deleted attributes */ test_attr_bug3(my_fcpl, my_fapl); /* Test "self referential" attributes */ + test_attr_bug4(my_fcpl, my_fapl); /* Test attributes on named datatypes */ } /* end for */ } /* end if */ else { @@ -10064,6 +10147,7 @@ test_attr(void) test_attr_bug1(fcpl, my_fapl); /* Test odd allocation operations */ test_attr_bug2(fcpl, my_fapl); /* Test many deleted attributes */ test_attr_bug3(fcpl, my_fapl); /* Test "self referential" attributes */ + test_attr_bug4(fcpl, my_fapl); /* Test attributes on named datatypes */ } /* end else */ } /* end for */ |