[svn-r19240] Merge revision 19077 to 1.8.5 branch: bug with local heaps and H5Pset_sizes.

Tested: fedora

MANIFEST

@@ -826,6 +826,7 @@
 ./test/gen_old_group.c			_DO_NOT_DISTRIBUTE_
 ./test/gen_old_layout.c			_DO_NOT_DISTRIBUTE_
 ./test/gen_old_mtime.c			_DO_NOT_DISTRIBUTE_
+./test/gen_sizes_lheap.c        _DO_NOT_DISTRIBUTE_
 ./test/gen_udlinks.c			_DO_NOT_DISTRIBUTE_
 ./test/getname.c
 ./test/gheap.c
@@ -886,6 +887,7 @@
 ./test/trefer.c
 ./test/trefstr.c
 ./test/tselect.c
+./test/tsizeslheap.h5
 ./test/tskiplist.c
 ./test/tsohm.c
 ./test/ttst.c

release_docs/RELEASE.txt

@@ -150,6 +150,10 @@ Bug Fixes since HDF5-1.8.4
 
     Library
     -------
+    - Fixed a bug that could cause file corruption when using non-default sizes
+      of addresses and/or lengths.  This bug could also cause uncorrupted files
+      with this property to be unreadable.  This bug was introduced in 1.8.5.
+      (NAF - 2010/07/16 - 1951)
     - valgrind reported an error of copying data to itself when a new attribute
       is written (Bug #1956).  I fixed it by taking out the memcpy step in the
       attribute code. (SLU - 2010/07/28)

src/H5HLcache.c

@@ -328,6 +328,11 @@ H5HL_prefix_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *_udata)
 
             /* Check if the current buffer from the speculative read already has the heap data */
             if(spec_read_size >= (heap->prfx_size + heap->dblk_size)) {
+                /* Set p to the start of the data block.  This is necessary
+                 * because there may be a gap between the used portion of the
+                 * prefix and the data block due to alignment constraints. */
+                p = buf + heap->prfx_size;
+
                 /* Copy the heap data from the speculative read buffer */
                 HDmemcpy(heap->dblk_image, p, heap->dblk_size);
             } /* end if */
@@ -433,6 +438,11 @@ H5HL_prefix_flush(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr,
 
         /* Check if the local heap is a single object in cache */
         if(heap->single_cache_obj) {
+            /* Set p to the start of the data block.  This is necessary because
+             * there may be a gap between the used portion of the prefix and the
+             * data block due to alignment constraints. */
+            p = buf + heap->prfx_size;
+
             /* Serialize the free list into the heap data's image */
             H5HL_fl_serialize(heap);
 

test/Makefile.am

@@ -62,7 +62,7 @@ check_PROGRAMS=$(TEST_PROG) error_test err_compat tcheck_version testmeta
 # so do not appear in this list.
 BUILD_ALL_PROGS=gen_bad_ohdr gen_bogus gen_cross gen_deflate gen_filters gen_idx \
     gen_new_array gen_new_fill gen_new_group gen_new_mtime gen_new_super \
-    gen_noencoder gen_nullspace gen_udlinks space_overflow
+    gen_noencoder gen_nullspace gen_udlinks space_overflow gen_sizes_lheap
 
 if BUILD_ALL_CONDITIONAL
   noinst_PROGRAMS=$(BUILD_ALL_PROGS)

test/Makefile.in

@@ -94,7 +94,7 @@ am__EXEEXT_2 = gen_bad_ohdr$(EXEEXT) gen_bogus$(EXEEXT) \
 	gen_new_group$(EXEEXT) gen_new_mtime$(EXEEXT) \
 	gen_new_super$(EXEEXT) gen_noencoder$(EXEEXT) \
 	gen_nullspace$(EXEEXT) gen_udlinks$(EXEEXT) \
-	space_overflow$(EXEEXT)
+	space_overflow$(EXEEXT) gen_sizes_lheap$(EXEEXT)
 PROGRAMS = $(noinst_PROGRAMS)
 app_ref_SOURCES = app_ref.c
 app_ref_OBJECTS = app_ref.$(OBJEXT)
@@ -240,6 +240,10 @@ gen_nullspace_SOURCES = gen_nullspace.c
 gen_nullspace_OBJECTS = gen_nullspace.$(OBJEXT)
 gen_nullspace_LDADD = $(LDADD)
 gen_nullspace_DEPENDENCIES = libh5test.la $(LIBHDF5)
+gen_sizes_lheap_SOURCES = gen_sizes_lheap.c
+gen_sizes_lheap_OBJECTS = gen_sizes_lheap.$(OBJEXT)
+gen_sizes_lheap_LDADD = $(LDADD)
+gen_sizes_lheap_DEPENDENCIES = libh5test.la $(LIBHDF5)
 gen_udlinks_SOURCES = gen_udlinks.c
 gen_udlinks_OBJECTS = gen_udlinks.$(OBJEXT)
 gen_udlinks_LDADD = $(LDADD)
@@ -366,12 +370,12 @@ SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c btree2.c \
 	flush2.c freespace.c gen_bad_ohdr.c gen_bogus.c gen_cross.c \
 	gen_deflate.c gen_filters.c gen_idx.c gen_new_array.c \
 	gen_new_fill.c gen_new_group.c gen_new_mtime.c gen_new_super.c \
-	gen_noencoder.c gen_nullspace.c gen_udlinks.c getname.c \
-	gheap.c hyperslab.c istore.c lheap.c links.c mf.c mount.c \
-	mtime.c ntypes.c objcopy.c ohdr.c pool.c reserved.c \
-	set_extent.c space_overflow.c stab.c tcheck_version.c \
-	$(testhdf5_SOURCES) testmeta.c $(ttsafe_SOURCES) unlink.c \
-	vfd.c
+	gen_noencoder.c gen_nullspace.c gen_sizes_lheap.c \
+	gen_udlinks.c getname.c gheap.c hyperslab.c istore.c lheap.c \
+	links.c mf.c mount.c mtime.c ntypes.c objcopy.c ohdr.c pool.c \
+	reserved.c set_extent.c space_overflow.c stab.c \
+	tcheck_version.c $(testhdf5_SOURCES) testmeta.c \
+	$(ttsafe_SOURCES) unlink.c vfd.c
 DIST_SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c \
 	btree2.c cache.c cache_api.c cmpd_dset.c cross_read.c dangle.c \
 	dsets.c dt_arith.c dtransform.c dtypes.c enum.c err_compat.c \
@@ -379,12 +383,12 @@ DIST_SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c \
 	flush2.c freespace.c gen_bad_ohdr.c gen_bogus.c gen_cross.c \
 	gen_deflate.c gen_filters.c gen_idx.c gen_new_array.c \
 	gen_new_fill.c gen_new_group.c gen_new_mtime.c gen_new_super.c \
-	gen_noencoder.c gen_nullspace.c gen_udlinks.c getname.c \
-	gheap.c hyperslab.c istore.c lheap.c links.c mf.c mount.c \
-	mtime.c ntypes.c objcopy.c ohdr.c pool.c reserved.c \
-	set_extent.c space_overflow.c stab.c tcheck_version.c \
-	$(testhdf5_SOURCES) testmeta.c $(ttsafe_SOURCES) unlink.c \
-	vfd.c
+	gen_noencoder.c gen_nullspace.c gen_sizes_lheap.c \
+	gen_udlinks.c getname.c gheap.c hyperslab.c istore.c lheap.c \
+	links.c mf.c mount.c mtime.c ntypes.c objcopy.c ohdr.c pool.c \
+	reserved.c set_extent.c space_overflow.c stab.c \
+	tcheck_version.c $(testhdf5_SOURCES) testmeta.c \
+	$(ttsafe_SOURCES) unlink.c vfd.c
 ETAGS = etags
 CTAGS = ctags
 am__tty_colors = \
@@ -710,7 +714,7 @@ TEST_PROG = testhdf5 lheap ohdr stab gheap cache cache_api \
 # so do not appear in this list.
 BUILD_ALL_PROGS = gen_bad_ohdr gen_bogus gen_cross gen_deflate gen_filters gen_idx \
     gen_new_array gen_new_fill gen_new_group gen_new_mtime gen_new_super \
-    gen_noencoder gen_nullspace gen_udlinks space_overflow
+    gen_noencoder gen_nullspace gen_udlinks space_overflow gen_sizes_lheap
 
 
 # The libh5test library provides common support code for the tests.
@@ -931,6 +935,9 @@ gen_noencoder$(EXEEXT): $(gen_noencoder_OBJECTS) $(gen_noencoder_DEPENDENCIES)
 gen_nullspace$(EXEEXT): $(gen_nullspace_OBJECTS) $(gen_nullspace_DEPENDENCIES) 
 	@rm -f gen_nullspace$(EXEEXT)
 	$(LINK) $(gen_nullspace_OBJECTS) $(gen_nullspace_LDADD) $(LIBS)
+gen_sizes_lheap$(EXEEXT): $(gen_sizes_lheap_OBJECTS) $(gen_sizes_lheap_DEPENDENCIES) 
+	@rm -f gen_sizes_lheap$(EXEEXT)
+	$(LINK) $(gen_sizes_lheap_OBJECTS) $(gen_sizes_lheap_LDADD) $(LIBS)
 gen_udlinks$(EXEEXT): $(gen_udlinks_OBJECTS) $(gen_udlinks_DEPENDENCIES) 
 	@rm -f gen_udlinks$(EXEEXT)
 	$(LINK) $(gen_udlinks_OBJECTS) $(gen_udlinks_LDADD) $(LIBS)
@@ -1047,6 +1054,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_new_super.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_noencoder.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_nullspace.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_sizes_lheap.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_udlinks.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/getname.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gheap.Po@am__quote@

test/gen_sizes_lheap.c

@@ -0,0 +1,82 @@
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Copyright by The HDF Group.                                               *
+ * Copyright by the Board of Trustees of the University of Illinois.         *
+ * All rights reserved.                                                      *
+ *                                                                           *
+ * This file is part of HDF5.  The full HDF5 copyright notice, including     *
+ * terms governing use, modification, and redistribution, is contained in    *
+ * the files COPYING and Copyright.html.  COPYING can be found at the root   *
+ * of the source code distribution tree; Copyright.html can be found at the  *
+ * root level of an installed copy of the electronic HDF5 document set and   *
+ * is linked from the top-level documents page.  It can also be found at     *
+ * http://hdfgroup.org/HDF5/doc/Copyright.html.  If you do not have          *
+ * access to either file, you may request a copy from help@hdfgroup.org.     *
+ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+/*
+ * Programmer:  Neil Fortner <nfortne2@hdfgroup.org>
+ *              Thursday, July  15, 2010
+ *
+ * Purpose:     Creates a file with non-default sizes of lengths and addresses.
+ *              This is used to make sure that the local heap code is able to
+ *              handle this case correctly, even when the heap prefix and data
+ *              are contiguous.
+ */
+
+#include "hdf5.h"
+
+#define TESTFILE   "tsizeslheap.h5"
+
+
+/*-------------------------------------------------------------------------
+ * Function:    main
+ *
+ * Purpose:
+ *
+ * Return:      Success:
+ *
+ *              Failure:
+ *
+ * Programmer:  Neil Fortner
+ *              Thursday, July  15, 2010
+ *
+ * Modifications:
+ *
+ *-------------------------------------------------------------------------
+ */
+int
+main(void)
+{
+    hid_t       file, space, dset, fcpl;
+
+    /* Create the FCPL */
+    fcpl = H5Pcreate(H5P_FILE_CREATE);
+    if(fcpl < 0)
+        printf("fcpl < 0!\n");
+
+    /* Set sizeof_addr and sizeof_size to be 4 */
+    if(H5Pset_sizes(fcpl, 4, 4) < 0)
+        printf("H5Pset_sizes < 0!\n");
+
+    /* Create the file */
+    file = H5Fcreate(TESTFILE, H5F_ACC_TRUNC, fcpl, H5P_DEFAULT);
+    if(file < 0)
+        printf("file < 0!\n");
+
+    /* Create the dataspace (for dataset) */
+    space = H5Screate(H5S_SCALAR);
+    if(space < 0)
+        printf("space < 0!\n");
+
+    /* Create the dataset with compound array fields */
+    dset = H5Dcreate2(file, "Dataset1", H5T_NATIVE_INT, space, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+    if(dset < 0)
+        printf("dset < 0!\n");
+
+    H5Dclose(dset);
+    H5Sclose(space);
+    H5Fclose(file);
+    H5Pclose(fcpl);
+
+    return 0;
+}

test/lheap.c

@@ -20,6 +20,7 @@
  * Purpose:	Test local heaps used by symbol tables (groups).
  */
 #include "h5test.h"
+#include "H5srcdir.h"
 #include "H5ACprivate.h"
 #include "H5HLprivate.h"
 #include "H5Iprivate.h"
@@ -29,6 +30,8 @@ const char *FILENAME[] = {
     NULL
 };
 
+#define TESTFILE "tsizeslheap.h5"
+
 #define NOBJS   40
 
 
@@ -160,6 +163,29 @@ main(void)
 
     if (H5Fclose(file)<0) goto error;
     PASSED();
+
+    /* Check opening existing file non-default sizes of lengths and addresses */
+    TESTING("opening pre-created file with non-default sizes");
+    {
+        const char *testfile = H5_get_srcdir_filename(TESTFILE); /* Corrected test file name */
+        hid_t dset = -1;
+
+        file = H5Fopen(testfile, H5F_ACC_RDONLY, H5P_DEFAULT);
+        if(file >= 0){
+            if((dset = H5Dopen2(file, "/Dataset1", H5P_DEFAULT)) < 0)
+                TEST_ERROR
+            if(H5Dclose(dset) < 0) TEST_ERROR
+            if(H5Fclose(file) < 0) TEST_ERROR
+        }
+        else {
+            H5_FAILED();
+            printf("***cannot open the pre-created non-default sizes test file (%s)\n",
+                testfile);
+            goto error;
+        } /* end else */
+    }
+    PASSED();
+
     puts("All local heap tests passed.");
     h5_cleanup(FILENAME, fapl);