summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNeil Fortner <nfortne2@hdfgroup.org>2008-10-24 01:05:41 (GMT)
committerNeil Fortner <nfortne2@hdfgroup.org>2008-10-24 01:05:41 (GMT)
commit220c8585e1af4bcff9595ff1506ad90b8c4a4736 (patch)
tree8b309320c988de13466acbbf9e56b294cf4fa972
parent53a8b67778122850eb960fd4f74cc42dee1baf11 (diff)
downloadhdf5-220c8585e1af4bcff9595ff1506ad90b8c4a4736.zip
hdf5-220c8585e1af4bcff9595ff1506ad90b8c4a4736.tar.gz
hdf5-220c8585e1af4bcff9595ff1506ad90b8c4a4736.tar.bz2
[svn-r15941] Purpose: Close bug 1305.
Description: Added a new function, H5O_alloc_shrink_chunk, which removes all null messages from a chunk and shrinks the chunk appropriately. Modified H5O_merge_null to call this function when a null message is created with a size >= 64k. Tests added for this functionality. Tested: kagiso (h5committest on 1.8 version)
-rw-r--r--release_docs/RELEASE.txt2
-rw-r--r--src/H5Oalloc.c186
-rw-r--r--test/tattr.c166
3 files changed, 349 insertions, 5 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index fa747a6..9709d34 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -117,6 +117,8 @@ Bug Fixes since HDF5-1.8.0 release
Library
-------
+ - Fixed an error where a null message could be created that was larger
+ than could be written to the file. (NAF - 2008/10/23)
- Corrected error with family/split/multi VFD not updating driver info
when "latest" version of the file format used. (QAK - 2008/10/14)
- Corrected alignment+threshold errors to work correctly when metadata
diff --git a/src/H5Oalloc.c b/src/H5Oalloc.c
index 3234d82..7c6cf9d 100644
--- a/src/H5Oalloc.c
+++ b/src/H5Oalloc.c
@@ -69,8 +69,9 @@ static htri_t H5O_alloc_extend_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh,
static unsigned H5O_alloc_new_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh,
size_t size);
static htri_t H5O_move_msgs_forward(H5O_t *oh);
-static htri_t H5O_merge_null(H5O_t *oh);
+static htri_t H5O_merge_null(H5F_t *f, H5O_t *oh, hid_t dxpl_id);
static htri_t H5O_remove_empty_chunks(H5F_t *f, H5O_t *oh, hid_t dxpl_id);
+static herr_t H5O_alloc_shrink_chunk(H5F_t *f, H5O_t *oh, hid_t dxpl_id, unsigned chunkno);
/*********************/
@@ -1331,13 +1332,13 @@ done:
*-------------------------------------------------------------------------
*/
static htri_t
-H5O_merge_null(H5O_t *oh)
+H5O_merge_null(H5F_t *f, H5O_t *oh, hid_t dxpl_id)
{
hbool_t merged_msg; /* Flag to indicate that messages were merged */
hbool_t did_merging = FALSE; /* Whether any messages were merged */
htri_t ret_value; /* Return value */
- FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5O_merge_null)
+ FUNC_ENTER_NOAPI_NOINIT(H5O_merge_null)
/* check args */
HDassert(oh != NULL);
@@ -1400,6 +1401,11 @@ H5O_merge_null(H5O_t *oh)
/* (Don't bother reducing size of message array for now -QAK) */
oh->nmesgs--;
+ /* If the merged message is too large, shrink the chunk */
+ if(curr_msg->raw_size >= H5O_MESG_MAX_SIZE)
+ if(H5O_alloc_shrink_chunk(f, oh, dxpl_id, curr_msg->chunkno) < 0)
+ HGOTO_ERROR(H5E_OHDR, H5E_CANTPACK, FAIL, "unable to shrink chunk")
+
/* Get out of loop */
break;
} /* end if */
@@ -1420,6 +1426,7 @@ H5O_merge_null(H5O_t *oh)
/* Set return value */
ret_value = did_merging;
+done:
FUNC_LEAVE_NOAPI(ret_value)
} /* H5O_merge_null() */
@@ -1603,7 +1610,7 @@ H5O_condense_header(H5F_t *f, H5O_t *oh, hid_t dxpl_id)
rescan_header = TRUE;
/* Scan for adjacent null messages & merge them */
- result = H5O_merge_null(oh);
+ result = H5O_merge_null(f, oh, dxpl_id);
if(result < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTPACK, FAIL, "can't pack null header messages")
if(result > 0)
@@ -1624,3 +1631,174 @@ done:
FUNC_LEAVE_NOAPI(ret_value)
} /* H5O_condense_header() */
+
+/*-------------------------------------------------------------------------
+ *
+ * Function: H5O_alloc_shrink_chunk
+ *
+ * Purpose: Shrinks a chunk, removing all null messages and any gap.
+ *
+ * Return: Non-negative on success/Negative on failure
+ *
+ * Programmer: Neil Fortner
+ * nfortne2@hdfgroup.org
+ * Oct 20 2008
+ *
+ *-------------------------------------------------------------------------
+ */
+static herr_t
+H5O_alloc_shrink_chunk(H5F_t *f,
+ H5O_t *oh,
+ hid_t dxpl_id,
+ unsigned chunkno)
+{
+ H5O_chunk_t *chunk = &oh->chunk[chunkno]; /* Chunk to shrink */
+ H5O_mesg_t *curr_msg;
+ 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 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 */
+ herr_t ret_value = SUCCEED; /* Return value */
+ unsigned u; /* Index */
+
+ FUNC_ENTER_NOAPI_NOINIT(H5O_alloc_shrink_chunk)
+
+ /* check args */
+ HDassert(f != NULL);
+
+ /* Loop backwards to increase the chance of seeing more null messages at the
+ * end of the chunk. Note that we rely on unsigned u wrapping around at the
+ * end.
+ */
+ 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 */
+
+ /* Adjust the new chunk size */
+ new_size -= shrink_size;
+
+ /* 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));
+
+ /* Decrement # of messages */
+ /* (Don't bother reducing size of message array for now) */
+ oh->nmesgs--;
+ } /* end else */
+ } /* end if */
+ } /* end for */
+
+ /* 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;
+ new_size_flags = H5O_HDR_CHUNK0_1;
+ adjust_size_flags = TRUE;
+ } /* end if */
+ /* Check for moving to a 2-byte size encoding */
+ else if (orig_prfx_size > 2 && chunk0_newsize <= 65535) {
+ less_prfx_size = orig_prfx_size - 2;
+ new_size_flags = H5O_HDR_CHUNK0_2;
+ adjust_size_flags = TRUE;
+ } /* end if */
+ /* Check for moving to a 4-byte size encoding */
+ else if (orig_prfx_size > 4 && chunk0_newsize <= 4294967295) {
+ less_prfx_size = orig_prfx_size - 4;
+ new_size_flags = H5O_HDR_CHUNK0_4;
+ adjust_size_flags = TRUE;
+ } /* end if */
+ } /* end if */
+
+ if(adjust_size_flags) {
+ /* Adjust object header prefix flags */
+ oh->flags &= ~H5O_HDR_CHUNK0_SIZE;
+ oh->flags |= new_size_flags;
+
+ /* Slide chunk 0 data down */
+ HDmemmove(chunk->image + H5O_SIZEOF_HDR(oh) - sizeof_chksum,
+ chunk->image + H5O_SIZEOF_HDR(oh) - sizeof_chksum + less_prfx_size,
+ new_size - H5O_SIZEOF_HDR(oh));
+
+ /* Adjust chunk size */
+ new_size -= less_prfx_size;
+ } /* end if */
+
+ /* Allocate less memory space for chunk's image */
+ chunk->size = new_size;
+ chunk->image = H5FL_BLK_REALLOC(chunk_image, old_image, chunk->size);
+ chunk->gap = 0;
+ chunk->dirty = TRUE;
+ if (NULL == oh->chunk[chunkno].image)
+ HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed")
+
+ /* Spin through existing messages, adjusting them */
+ for (u = 0, curr_msg = &oh->mesg[0]; u < oh->nmesgs; u++, curr_msg++) {
+ if (adjust_size_flags || (chunk->image != old_image))
+ /* Adjust raw addresses for messages in this chunk to reflect new 'image' address */
+ if (curr_msg->chunkno == chunkno)
+ curr_msg->raw = chunk->image - less_prfx_size + (curr_msg->raw - old_image);
+
+ /* Find continuation message which points to this chunk and adjust chunk's size */
+ /* (Chunk 0 doesn't have a continuation message that points to it and
+ * its size is directly encoded in the object header) */
+ if (chunkno > 0 && (H5O_CONT_ID == curr_msg->type->id) &&
+ (((H5O_cont_t *)(curr_msg->native))->chunkno == chunkno)) {
+ /* Adjust size of continuation message */
+ HDassert(((H5O_cont_t *)(curr_msg->native))->size == old_size);
+ ((H5O_cont_t *)(curr_msg->native))->size = chunk->size;
+
+ /* Flag continuation message as dirty */
+ curr_msg->dirty = TRUE;
+ } /* end if */
+ } /* end for */
+
+ /* 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")
+
+done:
+ FUNC_LEAVE_NOAPI(ret_value)
+} /* H5O_alloc_shrink_chunk() */
diff --git a/test/tattr.c b/test/tattr.c
index 953a50f..fe092e6 100644
--- a/test/tattr.c
+++ b/test/tattr.c
@@ -132,6 +132,9 @@ float attr_data5=(float)-5.123; /* Test data for 5th attribute */
#define NATTR_MANY_OLD 350
#define NATTR_MANY_NEW 35000
+#define BUG2_NATTR 100
+#define BUG2_NATTR2 16
+
/* Attribute iteration struct */
typedef struct {
H5_iter_order_t order; /* Direction of iteration */
@@ -1781,7 +1784,7 @@ test_attr_dtype_shared(hid_t fapl)
** one ID handles.
**
****************************************************************/
-static int
+static void
test_attr_duplicate_ids(hid_t fapl)
{
hid_t fid1; /* HDF5 File IDs */
@@ -9611,6 +9614,165 @@ test_attr_bug1(hid_t fcpl, hid_t fapl)
/****************************************************************
**
+** test_attr_bug2(): Test basic H5A (attribute) code.
+** Tests deleting a large number of attributes with the
+** intention of creating a null message with a size that
+** is too large. This routine deletes every other
+** attribute, but the original bug could also be
+** reproduced by deleting every attribute except a few to
+** keep the chunk open.
+**
+****************************************************************/
+static void
+test_attr_bug2(hid_t fcpl, hid_t fapl)
+{
+ hid_t fid; /* File ID */
+ hid_t gid; /* Group ID */
+ hid_t aid; /* Attribute ID */
+ hid_t sid; /* Dataspace ID */
+ hid_t tid; /* Datatype ID */
+ hid_t gcpl; /* Group creation property list */
+ hsize_t dims[2] = {10, 100}; /* Attribute dimensions */
+ char aname[4]; /* Attribute name */
+ unsigned i; /* index */
+ herr_t ret; /* Generic return status */
+ htri_t tri_ret; /* htri_t return status */
+
+ /* Output message about test being performed */
+ MESSAGE(5, ("Testing Allocating and De-allocating Attributes in Unusual Way\n"));
+
+ /* Create group creation property list */
+ gcpl = H5Pcreate(H5P_GROUP_CREATE);
+ CHECK(gcpl, FAIL, "H5Pcreate");
+
+ /* Prevent the library from switching to dense attribute storage */
+ /* Not doing this with the latest format actually triggers a different bug.
+ * This will be tested here as soon as it is fixed. -NAF
+ */
+ ret = H5Pset_attr_phase_change (gcpl, BUG2_NATTR+10, BUG2_NATTR+5);
+ CHECK(ret, FAIL, "H5Pset_attr_phase_change");
+
+ /* Create dataspace ID for attributes */
+ sid = H5Screate_simple(2, dims, NULL);
+ CHECK(sid, FAIL, "H5Screate_simple");
+
+ /* Create main group to operate on */
+ fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl, fapl);
+ CHECK(fid, FAIL, "H5Fcreate");
+
+ gid = H5Gcreate2(fid, GROUP1_NAME, H5P_DEFAULT, gcpl, H5P_DEFAULT);
+ CHECK(gid, FAIL, "H5Gcreate2");
+
+ /* Create attributes on group */
+ for (i=0; i<BUG2_NATTR; i++) {
+ sprintf(aname, "%03u", i);
+ aid = H5Acreate2(gid, aname, H5T_STD_I32LE, sid, H5P_DEFAULT, H5P_DEFAULT);
+ CHECK(aid, FAIL, "H5Acreate2");
+
+ ret = H5Aclose(aid);
+ CHECK(ret, FAIL, "H5Aclose");
+ }
+
+ /* Delete every other attribute */
+ for (i=1; i<BUG2_NATTR; i+=2) {
+ sprintf(aname, "%03u", i);
+ ret = H5Adelete(gid, aname);
+ CHECK(ret, FAIL, "H5Adelete");
+ }
+
+ /* Close IDs */
+ ret = H5Gclose(gid);
+ CHECK(ret, FAIL, "H5Gclose");
+
+ ret = H5Fclose(fid);
+ CHECK(ret, FAIL, "H5Fclose");
+
+ ret = H5Sclose(sid);
+ CHECK(ret, FAIL, "H5Sclose");
+
+ /* Reopen file and group */
+ fid = H5Fopen(FILENAME, H5F_ACC_RDONLY, fapl);
+ CHECK(fid, FAIL, "H5Fopen");
+
+ gid = H5Gopen2(fid, GROUP1_NAME, H5P_DEFAULT);
+ CHECK(gid, FAIL, "H5Gopen");
+
+ /* Open an attribute in the middle */
+ i = (BUG2_NATTR / 4) * 2;
+ sprintf(aname, "%03u", i);
+ aid = H5Aopen(gid, aname, H5P_DEFAULT);
+ CHECK(aid, FAIL, "H5Aopen");
+
+ /* Verify that the attribute has the correct datatype */
+ tid = H5Aget_type(aid);
+ CHECK(tid, FAIL, "H5Aget_type");
+
+ tri_ret = H5Tequal(tid, H5T_STD_I32LE);
+ VERIFY(tri_ret, TRUE, "H5Tequal");
+
+ /* Close IDs */
+ ret = H5Tclose(tid);
+ CHECK(ret, FAIL, "H5Tclose");
+
+ ret = H5Aclose(aid);
+ CHECK(ret, FAIL, "H5Aclose");
+
+ ret = H5Gclose(gid);
+ CHECK(ret, FAIL, "H5Gclose");
+
+ ret = H5Fclose(fid);
+ CHECK(ret, FAIL, "H5Fclose");
+
+ /* Now test a variation on this bug - where either the size of chunk 0 goes
+ * down a "notch" or two, or chunk 1 becomes completely null at the same
+ * time that a null message that is too large is formed */
+ dims[0] = 25;
+ dims[1] = 41; /* 1025*4 byte attribute size */
+
+ /* Create dataspace ID for attributes */
+ sid = H5Screate_simple(2, dims, NULL);
+ CHECK(sid, FAIL, "H5Screate_simple");
+
+ /* Create main group to operate on */
+ fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl, fapl);
+ CHECK(fid, FAIL, "H5Fcreate");
+
+ gid = H5Gcreate2(fid, GROUP1_NAME, H5P_DEFAULT, gcpl, H5P_DEFAULT);
+ CHECK(gid, FAIL, "H5Gcreate2");
+
+ /* Create attributes on group */
+ for (i=0; i<BUG2_NATTR2; i++) {
+ sprintf(aname, "%03u", i);
+ aid = H5Acreate2(gid, aname, H5T_STD_I32LE, sid, H5P_DEFAULT, H5P_DEFAULT);
+ CHECK(aid, FAIL, "H5Acreate2");
+
+ ret = H5Aclose(aid);
+ CHECK(ret, FAIL, "H5Aclose");
+ }
+
+ /* Delete every other attribute */
+ for (i=0; i<BUG2_NATTR2; i++) {
+ sprintf(aname, "%03u", i);
+ ret = H5Adelete(gid, aname);
+ CHECK(ret, FAIL, "H5Adelete");
+ }
+
+ /* Close IDs */
+ ret = H5Gclose(gid);
+ CHECK(ret, FAIL, "H5Gclose");
+
+ ret = H5Fclose(fid);
+ CHECK(ret, FAIL, "H5Fclose");
+
+ ret = H5Sclose(sid);
+ CHECK(ret, FAIL, "H5Sclose");
+
+ ret = H5Pclose(gcpl);
+ CHECK(ret, FAIL, "H5Pclose");
+} /* test_attr_bug2() */
+
+/****************************************************************
+**
** test_attr(): Main H5A (attribute) testing routine.
**
****************************************************************/
@@ -9750,6 +9912,7 @@ test_attr(void)
/* Tests that address specific bugs */
test_attr_bug1(my_fcpl, my_fapl); /* Test odd allocation operations */
+ test_attr_bug2(my_fcpl, my_fapl); /* Test many deleted attributes */
} /* end for */
} /* end if */
else {
@@ -9769,6 +9932,7 @@ test_attr(void)
/* Tests that address specific bugs */
test_attr_bug1(fcpl, my_fapl); /* Test odd allocation operations */
+ test_attr_bug2(fcpl, my_fapl); /* Test many deleted attributes */
} /* end else */
} /* end for */