[svn-r16475] Purpose: Fix rare error when adding a new object header message

Description:
Since the new object header format, it has been possible for a situation to be
created where none of the messages are large enough to hold a continuation
message and there are no null messages to merge with.  This makes it impossible
to add a new object header chunk.  This case will now be handled by moving every
message in the last chunk to the newly allocated one, except for null messages
which are deleted.

Tested: jam, smirom (h5committest)

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 */