[svn-r16376] Purpose: Fix a bug encountered when copying shared messages

Description: When attempting to copy an object with a message shared in its own
object header, the library attempts to protect the same object header twice.
Previously, it was possible for the object header to be protected with write
access in one or both of these protects, which would be illegal.  The library
should now always protect with read only access in this case.  The conditions
for fixing incorrect datatype versions have been made weaker to support this
change.  The version will only be corrected if the object header the datatype
is in is modified.

Tested: jam, smirom (h5committest)

8 files changed, 43 insertions, 14 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 4e35418..b225ae5 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -141,6 +141,8 @@ Bug Fixes since HDF5-1.8.0 release
 
     Library
     -------
+      - Fixed a bug that could cause problems when copying an object with a shared
+            message in its own object header. (NAF - 2009/01/29)
       - Changed H5Tset_order to properly reject H5T_ORDER_NONE for most
             datatypes. (NAF - 2009/01/27)
       - Fixed a bug where H5Tpack wouldn't remove trailing space from an
diff --git a/src/H5O.c b/src/H5O.c
index 52338cc..1d640fa 100644
--- a/src/H5O.c
+++ b/src/H5O.c
@@ -2273,8 +2273,7 @@ H5O_get_info(H5O_loc_t *oloc, hid_t dxpl_id, hbool_t want_ih_info, H5O_info_t *o
     HDassert(oinfo);
 
     /* Get the object header */
-    if(NULL == (oh = (H5O_t *)H5AC_protect(oloc->file, dxpl_id, H5AC_OHDR, oloc->addr, NULL, NULL,
-        (H5F_get_intent(oloc->file) & H5F_ACC_RDWR) ? H5AC_WRITE : H5AC_READ)))
+    if(NULL == (oh = (H5O_t *)H5AC_protect(oloc->file, dxpl_id, H5AC_OHDR, oloc->addr, NULL, NULL, H5AC_READ)))
 	HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "unable to load object header")
 
     /* Reset the object info structure */
diff --git a/src/H5Oattribute.c b/src/H5Oattribute.c
index 846d2ec..ea9a7f5 100644
--- a/src/H5Oattribute.c
+++ b/src/H5Oattribute.c
@@ -1207,7 +1207,7 @@ H5O_attr_iterate_real(hid_t loc_id, const H5O_loc_t *loc, hid_t dxpl_id,
 
     /* Protect the object header to iterate over */
     if(NULL == (oh = (H5O_t *)H5AC_protect(loc->file, dxpl_id, H5AC_OHDR, loc->addr,
-        NULL, NULL, (H5F_get_intent(loc->file) & H5F_ACC_RDWR) ? H5AC_WRITE : H5AC_READ)))
+        NULL, NULL, H5AC_READ)))
 	HGOTO_ERROR(H5E_ATTR, H5E_CANTLOAD, FAIL, "unable to load object header")
 
     /* Check for attribute info stored */
diff --git a/src/H5Ocache.c b/src/H5Ocache.c
index e5a772f..db093a4 100644
--- a/src/H5Ocache.c
+++ b/src/H5Ocache.c
@@ -880,12 +880,21 @@ HDfprintf(stderr, "%s: oh->cache_info.free_file_space_on_destroy = %t\n", FUNC,
     /* destroy messages */
     if(oh->mesg) {
         for(u = 0; u < oh->nmesgs; u++) {
-            /* Verify that message is clean */
-            HDassert(oh->mesg[u].dirty == 0);
+            /* Verify that message is clean, unless it could have been marked
+             * dirty by decoding */
+#ifndef NDEBUG
+            if(oh->ndecode_dirtied && oh->mesg[u].dirty)
+                oh->ndecode_dirtied--;
+            else
+                HDassert(oh->mesg[u].dirty == 0);
+#endif /* NDEBUG */
 
             H5O_msg_free_mesg(&oh->mesg[u]);
         } /* end for */
 
+        /* Make sure we accounted for all the messages dirtied by decoding */
+        HDassert(!oh->ndecode_dirtied);
+
         oh->mesg = (H5O_mesg_t *)H5FL_SEQ_FREE(H5O_mesg_t, oh->mesg);
     } /* end if */
 
@@ -929,6 +938,11 @@ H5O_clear(H5F_t *f, H5O_t *oh, hbool_t destroy)
     for(u = 0; u < oh->nmesgs; u++)
         oh->mesg[u].dirty = FALSE;
 
+#ifndef NDEBUG
+        /* Reset the number of messages dirtied by decoding */
+        oh->ndecode_dirtied = 0;
+#endif /* NDEBUG */
+
     /* Mark whole header as clean */
     oh->cache_info.is_dirty = FALSE;
 
diff --git a/src/H5Ocopy.c b/src/H5Ocopy.c
index 9a905d2..5334539 100644
--- a/src/H5Ocopy.c
+++ b/src/H5Ocopy.c
@@ -315,8 +315,7 @@ H5O_copy_header_real(const H5O_loc_t *oloc_src, H5O_loc_t *oloc_dst /*out */,
     HDassert(cpy_info);
 
     /* Get source object header */
-    if(NULL == (oh_src = (H5O_t *)H5AC_protect(oloc_src->file, dxpl_id, H5AC_OHDR, oloc_src->addr, NULL, NULL,
-        (H5F_get_intent(oloc_src->file) & H5F_ACC_RDWR) ? H5AC_WRITE : H5AC_READ)))
+    if(NULL == (oh_src = (H5O_t *)H5AC_protect(oloc_src->file, dxpl_id, H5AC_OHDR, oloc_src->addr, NULL, NULL, H5AC_READ)))
         HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "unable to load object header")
 
     /* Get pointer to object class for this object */
diff --git a/src/H5Omessage.c b/src/H5Omessage.c
index 1682117..5a0577a 100644
--- a/src/H5Omessage.c
+++ b/src/H5Omessage.c
@@ -484,8 +484,7 @@ H5O_msg_read(const H5O_loc_t *loc, unsigned type_id, void *mesg,
     HDassert(type_id < NELMTS(H5O_msg_class_g));
 
     /* Get the object header */
-    if(NULL == (oh = (H5O_t *)H5AC_protect(loc->file, dxpl_id, H5AC_OHDR, loc->addr, NULL, NULL,
-        (H5F_get_intent(loc->file) & H5F_ACC_RDWR) ? H5AC_WRITE : H5AC_READ)))
+    if(NULL == (oh = (H5O_t *)H5AC_protect(loc->file, dxpl_id, H5AC_OHDR, loc->addr, NULL, NULL, H5AC_READ)))
 	HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, NULL, "unable to load object header")
 
     /* Call the "real" read routine */
@@ -1231,8 +1230,7 @@ H5O_msg_iterate(const H5O_loc_t *loc, unsigned type_id,
     HDassert(op);
 
     /* Protect the object header to iterate over */
-    if(NULL == (oh = (H5O_t *)H5AC_protect(loc->file, dxpl_id, H5AC_OHDR, loc->addr, NULL, NULL,
-        (H5F_get_intent(loc->file) & H5F_ACC_RDWR) ? H5AC_WRITE : H5AC_READ)))
+    if(NULL == (oh = (H5O_t *)H5AC_protect(loc->file, dxpl_id, H5AC_OHDR, loc->addr, NULL, NULL, H5AC_READ)))
 	HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "unable to load object header")
 
     /* Call the "real" iterate routine */
@@ -2229,6 +2227,12 @@ H5O_flush_msgs(H5F_t *f, H5O_t *oh)
     if(oh->nmesgs != u)
         HGOTO_ERROR(H5E_OHDR, H5E_CANTFLUSH, FAIL, "corrupt object header - too few messages")
 
+#ifndef NDEBUG
+        /* Reset the number of messages dirtied by decoding, as they have all
+         * been flushed */
+        oh->ndecode_dirtied = 0;
+#endif /* NDEBUG */
+
 done:
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5O_flush_msgs() */
diff --git a/src/H5Opkg.h b/src/H5Opkg.h
index f15198e..b7d4202 100644
--- a/src/H5Opkg.h
+++ b/src/H5Opkg.h
@@ -171,6 +171,13 @@
 #define H5O_DECODEIO_NOCHANGE           0x01u   /* IN: do not modify values */
 #define H5O_DECODEIO_DIRTY              0x02u   /* OUT: message has been changed */
 
+/* Macro to incremend ndecode_dirtied (only if we are debugging) */
+#ifndef NDEBUG
+#define INCR_NDECODE_DIRTIED(OH) (OH)->ndecode_dirtied++;
+#else /* NDEBUG */
+#define INCR_NDECODE_DIRTIED(OH) ;
+#endif /* NDEBUG */
+
 /* Load native information for a message, if it's not already present */
 /* (Only works for messages with decode callback) */
 #define H5O_LOAD_NATIVE(F, DXPL, IOF, OH, MSG, ERR)                           \
@@ -183,11 +190,12 @@
         if(NULL == ((MSG)->native = (msg_type->decode)((F), (DXPL), (MSG)->flags, &ioflags, (MSG)->raw))) \
             HGOTO_ERROR(H5E_OHDR, H5E_CANTDECODE, ERR, "unable to decode message") \
                                                                               \
-        /* Mark the object header dirty if the message was changed by decoding */ \
+        /* Mark the message dirty if it was changed by decoding */            \
         if((ioflags & H5O_DECODEIO_DIRTY) && (H5F_get_intent((F)) & H5F_ACC_RDWR)) { \
             (MSG)->dirty = TRUE;                                              \
-            if(H5AC_mark_pinned_or_protected_entry_dirty((F), (OH)) < 0)      \
-                HGOTO_ERROR(H5E_OHDR, H5E_CANTMARKDIRTY, ERR, "unable to mark object header as dirty") \
+            /* Increment the count of messages dirtied by decoding, but */    \
+            /* only ifndef NDEBUG */                                          \
+            INCR_NDECODE_DIRTIED(OH)                                          \
         }                                                                     \
                                                                               \
         /* Set the message's "shared info", if it's shareable */	      \
@@ -264,6 +272,9 @@ struct H5O_t {
                                          *      versions of the library)
                                          */
 #endif /* H5O_ENABLE_BAD_MESG_COUNT */
+#ifndef NDEBUG
+    size_t      ndecode_dirtied;        /* Number of messages dirtied by decoding */
+#endif /* NDEBUG */
 
     /* Object information (stored) */
     hbool_t     has_refcount_msg;       /* Whether the object has a ref. count message */
diff --git a/tools/testfiles/tfamily00000.h5 b/tools/testfiles/tfamily00000.h5
index a130bfd..35f9b49 100644
--- a/tools/testfiles/tfamily00000.h5
+++ b/tools/testfiles/tfamily00000.h5