From 00b9f24d9b161817876921a8bd983ed1d148def1 Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Mon, 9 Jan 2012 16:24:50 -0500 Subject: [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) --- release_docs/RELEASE.txt | 4 +- tools/h5repack/CMakeLists.txt | 107 +++++++++++++++++++++-------------------- tools/h5repack/h5repack.c | 10 ++-- tools/h5repack/h5repack.sh.in | 9 ++++ tools/h5repack/h5repack_refs.c | 17 +++++++ 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); -- cgit v0.12