[svn-r21869] Purpose:

  Fix for HDFFV-7840 h5repack: memory leak over one of the h5diff test file

Description:
  Turned out that there were two causes of memory leaks.
  1. for handling variable length string in attribute. 
  2. for handling compound type with non-reference members.  
  The first issue is fixed in copy_attr() which is updated to use h5tools_detect_vlen to take care of vlen string as well as vlen data. 
  The second is fixed in copy_refs_attr() of compound handling code.
  
Tested:
 jam (linux32-LE), koala (linux64-LE), heiwa (linuxppc64-BE), tejeda (mac32-LE), linew (solaris-BE),  Windows (32-LE), Cmake (jam)

5 files changed, 89 insertions, 58 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 8b22102..98e51a9 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -632,8 +632,10 @@ Bug Fixes since HDF5-1.8.0 release
 
     Tools
     -----
+        - h5repack: fixed memory leak for handling variable length string in 
+          attribute.  HDFFV-7840 (JKM 2012/01/06)
         - h5ls: fixed segfault when access region reference data in an
-          attribute. (JKM 2011/12/29)
+          attribute. HDFFV-7838 (JKM 2011/12/29)
         - h5diff: fixed segfault over non-comparable attribute with different 
           dimention or rank, along with '-c' option to display details.
           HDFFV-7770 (JKM 2011/10/24)
diff --git a/tools/h5repack/CMakeLists.txt b/tools/h5repack/CMakeLists.txt
index e38c272..8ea3c99 100644
--- a/tools/h5repack/CMakeLists.txt
+++ b/tools/h5repack/CMakeLists.txt
@@ -81,69 +81,64 @@ IF (BUILD_TESTING)
   ENDIF (HDF5_TEST_VFD)
     
   # --------------------------------------------------------------------
-  # Copy all the HDF5 files from the test directory into the source directory
+  # Copy all the HDF5 files from the source directory into the test directory
   # --------------------------------------------------------------------
-  SET (HDF5_REFERENCE_TEST_FILES
-      h5repack_ext.bin
-      ublock.bin
-      h5repack.info
-      h5repack_attr.h5
-      h5repack_attr_refs.h5
-      h5repack_deflate.h5
-      h5repack_early.h5
-      h5repack_ext.h5
-      h5repack_fill.h5
-      h5repack_filters.h5
-      h5repack_fletcher.h5
-      h5repack_hlink.h5
-      h5repack_layout.h5
-      h5repack_layouto.h5
-      h5repack_layout2.h5
-      h5repack_named_dtypes.h5
-      h5repack_nbit.h5
-      h5repack_objs.h5
-      h5repack_refs.h5
-      h5repack_shuffle.h5
-      h5repack_soffset.h5
-      h5repack_szip.h5
+  SET (LIST_HDF5_TEST_FILES
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_attr.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_attr_refs.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_deflate.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_early.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_ext.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_fill.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_filters.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_fletcher.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_hlink.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_layout.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_layouto.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_layout2.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_named_dtypes.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_nbit.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_objs.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_refs.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_shuffle.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_soffset.h5
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_szip.h5
+      # h5diff/testfile
+      ${HDF5_TOOLS_H5DIFF_SOURCE_DIR}/testfiles/h5diff_attr1.h5
+      # tools/testfiles
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00000.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00001.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00002.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00003.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00004.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00005.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00006.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00007.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00008.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00009.h5
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/tfamily00010.h5
   )
-  SET (HDF5_COMMON_TEST_FILES
-      tfamily00000.h5
-      tfamily00001.h5
-      tfamily00002.h5
-      tfamily00003.h5
-      tfamily00004.h5
-      tfamily00005.h5
-      tfamily00006.h5
-      tfamily00007.h5
-      tfamily00008.h5
-      tfamily00009.h5
-      tfamily00010.h5
-      h5repack_filters.h5.ddl
+
+  SET (LIST_OTHER_TEST_FILES
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_ext.bin
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/ublock.bin
+      ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack.info
+      # tools/testfiles
+      ${HDF5_TOOLS_SRC_DIR}/testfiles/h5repack_filters.h5.ddl
   )
 
-  FOREACH (h5_file ${HDF5_REFERENCE_TEST_FILES})
-    SET (dest "${PROJECT_BINARY_DIR}/testfiles/${h5_file}")
+  FOREACH (h5_file ${LIST_HDF5_TEST_FILES} ${LIST_OTHER_TEST_FILES})
+    GET_FILENAME_COMPONENT(fname "${h5_file}" NAME)
+    SET (dest "${PROJECT_BINARY_DIR}/testfiles/${fname}")
     #MESSAGE (STATUS " Copying ${h5_file}")
     ADD_CUSTOM_COMMAND (
         TARGET     h5repack
         POST_BUILD
         COMMAND    ${CMAKE_COMMAND}
-        ARGS       -E copy_if_different ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/${h5_file} ${dest}
+        ARGS       -E copy_if_different ${h5_file} ${dest}
     )
-  ENDFOREACH (h5_file ${HDF5_REFERENCE_TEST_FILES})
+  ENDFOREACH (h5_file ${LIST_HDF5_TEST_FILES} ${LIST_OTHER_TEST_FILES})
 
-  FOREACH (h5c_file ${HDF5_COMMON_TEST_FILES})
-    SET (dest "${PROJECT_BINARY_DIR}/testfiles/${h5c_file}")
-    #MESSAGE (STATUS " Copying ${h5_file}")
-    ADD_CUSTOM_COMMAND (
-        TARGET     h5repack
-        POST_BUILD
-        COMMAND    ${CMAKE_COMMAND}
-        ARGS       -E copy_if_different ${HDF5_TOOLS_SRC_DIR}/testfiles/${h5c_file} ${dest}
-    )
-  ENDFOREACH (h5c_file ${HDF5_COMMON_TEST_FILES})
-  
 ##############################################################################
 ##############################################################################
 ###           T H E   T E S T S  M A C R O S                               ###
@@ -336,6 +331,7 @@ IF (BUILD_TESTING)
           h5repack_ub.h5
           h5repack_ub_out.h5
           h5repack_attr_refs_out.h5
+          h5diff_attr1_out.h5
   )
   IF (NOT "${last_test}" STREQUAL "")
     SET_TESTS_PROPERTIES (H5REPACK-clearall-objects PROPERTIES DEPENDS ${last_test})
@@ -794,6 +790,13 @@ IF (BUILD_TESTING)
 # TODO: include this test when code portion is completed.
   ADD_H5_TEST (bug1797 "SKIP" ${FILE_ATTR_REF})
 
+# Add test for memory leak in attirbute. This test is verified by CTEST.   
+# 1. leak from vlen string  
+# 2. leak from compound type without reference member
+# (HDFFV-7840, )
+# Note: this test is experimental for sharing test file among tools
+  ADD_H5_TEST (HDFFV-7840 "TEST" h5diff_attr1.h5)
+
   IF (HDF5_TEST_VFD)
     # Run test with different Virtual File Driver
     FOREACH (vfd ${VFD_LIST})
diff --git a/tools/h5repack/h5repack.c b/tools/h5repack/h5repack.c
index 1f42eef..a5fb47f 100644
--- a/tools/h5repack/h5repack.c
+++ b/tools/h5repack/h5repack.c
@@ -544,9 +544,9 @@ int copy_attr(hid_t loc_in,
             if(H5Aclose(attr_out) < 0)
                 goto error;
 
-            /* Check if we have VL data in the attribute's  datatype that must
+            /* Check if we have VL data and string in the attribute's  datatype that must
              * be reclaimed */
-            if(TRUE == H5Tdetect_class(wtype_id, H5T_VLEN))
+            if (TRUE == h5tools_detect_vlen(wtype_id))
                 H5Dvlen_reclaim(wtype_id, space_id, H5P_DEFAULT, buf);
             HDfree(buf);
             buf = NULL;
@@ -576,13 +576,13 @@ int copy_attr(hid_t loc_in,
 error:
     H5E_BEGIN_TRY {
         if(buf) {
-            /* Check if we have VL data in the attribute's  datatype that must
+            /* Check if we have VL data and string in the attribute's  datatype that must
              * be reclaimed */
-            if(TRUE == H5Tdetect_class(wtype_id, H5T_VLEN))
+            if (TRUE == h5tools_detect_vlen(wtype_id))
                 H5Dvlen_reclaim(wtype_id, space_id, H5P_DEFAULT, buf);
 
             /* Free buf */
-            free(buf);
+            HDfree(buf);
         } /* end if */
 
         H5Tclose(ftype_id);
diff --git a/tools/h5repack/h5repack.sh.in b/tools/h5repack/h5repack.sh.in
index 88611ce..c1cbba1 100755
--- a/tools/h5repack/h5repack.sh.in
+++ b/tools/h5repack/h5repack.sh.in
@@ -101,6 +101,7 @@ $SRC_H5REPACK_TESTFILES/h5repack_refs.h5
 $SRC_H5REPACK_TESTFILES/h5repack_shuffle.h5
 $SRC_H5REPACK_TESTFILES/h5repack_soffset.h5
 $SRC_H5REPACK_TESTFILES/h5repack_szip.h5
+$SRC_H5DIFF_TESTFILES/h5diff_attr1.h5
 $SRC_TOOLS_TESTFILES/tfamily00000.h5
 $SRC_TOOLS_TESTFILES/tfamily00001.h5
 $SRC_TOOLS_TESTFILES/tfamily00002.h5
@@ -800,6 +801,14 @@ TOOLTEST h5repack_refs.h5
 # the references in attribute of compund or vlen datatype 
 TOOLTEST h5repack_attr_refs.h5 
 
+
+# Add test for memory leak in attirbute. This test is verified by CTEST.   
+# 1. leak from vlen string  
+# 2. leak from compound type without reference member
+# (HDFFV-7840, )
+# Note: this test is experimental for sharing test file among tools
+TOOLTEST h5diff_attr1.h5
+
 if test $nerrors -eq 0 ; then
     echo "All $TESTNAME tests passed."
     exit $EXIT_SUCCESS
diff --git a/tools/h5repack/h5repack_refs.c b/tools/h5repack/h5repack_refs.c
index f4aa8ff..5690091 100644
--- a/tools/h5repack/h5repack_refs.c
+++ b/tools/h5repack/h5repack_refs.c
@@ -533,6 +533,23 @@ static int copy_refs_attr(hid_t loc_in,
         		}
         		H5Tclose(mtid);
         	}
+            
+            /* if don't contain reference type, free malloc for continue 
+             * and malloc again later */
+            if (!ref_comp_field_n) 
+            {
+                if (ref_comp_index) 
+                {
+                	HDfree(ref_comp_index);
+                	ref_comp_index = NULL;
+                }
+
+                if (ref_comp_size) 
+                {
+            	    HDfree(ref_comp_size);
+        	        ref_comp_size = NULL;
+                }
+            }
         }
 
         is_ref_comp = (ref_comp_field_n > 0);