From 58ca19929fd614eddee99f0cdc95551d451b4a6a Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Tue, 22 Mar 2011 09:26:12 -0500 Subject: [svn-r20288] Purpose: Fixed Bug 2216 - GMQS: h5diff - memory leak when compares vlen string in dataset or attributes Description: Merged from HDF5 trunk r20266, r20270 and r20285. Test for dataset : valgrind --leak-check=full ./h5diff -v h5diff_dset1.h5 h5diff_dset2.h5 /g1/VLstring Test for attr : valgrind --leak-check=full ./h5diff h5diff_attr1.h5 h5diff_attr2.h5 Both test cases are in testing script. Tested: jam (linux32-LE), amani (linux64-LE), heiwa (linuxppc64-BE), Cmake - jam --- release_docs/RELEASE.txt | 4 ++ tools/h5dump/h5dump.c | 8 +--- tools/h5ls/h5ls.c | 4 +- tools/lib/h5diff_attr.c | 12 +++--- tools/lib/h5diff_dset.c | 94 ++++++++++++++++++++++++++-------------------- tools/lib/h5tools.c | 98 ++++++++++++++++++++++++++++++++++++++++-------- tools/lib/h5tools.h | 1 + 7 files changed, 150 insertions(+), 71 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index bfca1d0..20aa048 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -130,6 +130,10 @@ Bug Fixes since HDF5-1.8.6 ----- - Fixed memory leak for h5diff when accessing symbolic links with --follow-symlink option. Bug#2214 (JKM 2011/3/18) + - Fixed memory leak for h5diff when access variable length string + data. Bug#2216 (JKM 2011/3/18) + - Fixed memory leak for h5diff when accessing symbolic links with + --follow-symlink option. Bug#2214 (JKM 2011/3/18) - Fixed and improved help page for -a option of h5ls. Bug#1904 (JKM 2011/3/11) - Fixed h5copy to be able to copy any object into the same HDF5 file. diff --git a/tools/h5dump/h5dump.c b/tools/h5dump/h5dump.c index 2581f33..71b0804 100644 --- a/tools/h5dump/h5dump.c +++ b/tools/h5dump/h5dump.c @@ -2618,9 +2618,7 @@ dump_data(hid_t obj_id, int obj_data, struct subset_t *sset, int display_index) ndims = H5Sget_simple_extent_dims(space, size, NULL); /* Check if we have VL data in the dataset's datatype */ - if (h5tools_detect_vlen_str(p_type) == TRUE) - vl_data = TRUE; - if (H5Tdetect_class(p_type, H5T_VLEN) == TRUE) + if (h5tools_detect_vlen(p_type) == TRUE) vl_data = TRUE; for (i = 0; i < ndims; i++) @@ -5759,9 +5757,7 @@ xml_dump_data(hid_t obj_id, int obj_data, struct subset_t UNUSED * sset, int UNU p_type = h5tools_get_native_type(type); /* Check if we have VL data in the dataset's datatype */ - if (h5tools_detect_vlen_str(p_type) == TRUE) - vl_data = TRUE; - if (H5Tdetect_class(p_type, H5T_VLEN) == TRUE) + if (h5tools_detect_vlen(p_type) == TRUE) vl_data = TRUE; H5Tclose(type); diff --git a/tools/h5ls/h5ls.c b/tools/h5ls/h5ls.c index d288f4b..8987d03 100644 --- a/tools/h5ls/h5ls.c +++ b/tools/h5ls/h5ls.c @@ -1457,10 +1457,8 @@ list_attr(hid_t obj, const char *attr_name, const H5A_info_t UNUSED *ainfo, unsigned int vl_data = 0; /* contains VL datatypes */ /* Check if we have VL data in the dataset's datatype */ - if (h5tools_detect_vlen_str(p_type) == TRUE) + if (h5tools_detect_vlen(p_type) == TRUE) vl_data = TRUE; - if (H5Tdetect_class(p_type, H5T_VLEN) == TRUE) - vl_data = TRUE; temp_need= nelmts * MAX(H5Tget_size(type), H5Tget_size(p_type)); assert(temp_need == (hsize_t)((size_t)temp_need)); diff --git a/tools/lib/h5diff_attr.c b/tools/lib/h5diff_attr.c index 7e5fb04..097c809 100644 --- a/tools/lib/h5diff_attr.c +++ b/tools/lib/h5diff_attr.c @@ -221,12 +221,14 @@ hsize_t diff_attr(hid_t loc1_id, *---------------------------------------------------------------------- */ - /* Free buf1 and buf2, being careful to reclaim any VL data first */ - if(TRUE == H5Tdetect_class(mtype1_id, H5T_VLEN)) + /* Free buf1 and buf2, check both VLEN-data VLEN-string to reclaim any + * VLEN memory first */ + if(TRUE == h5tools_detect_vlen(mtype1_id)) H5Dvlen_reclaim(mtype1_id, space1_id, H5P_DEFAULT, buf1); HDfree(buf1); + buf1 = NULL; - if(TRUE == H5Tdetect_class(mtype2_id, H5T_VLEN)) + if(TRUE == h5tools_detect_vlen(mtype2_id)) H5Dvlen_reclaim(mtype2_id, space2_id, H5P_DEFAULT, buf2); HDfree(buf2); buf2 = NULL; @@ -256,12 +258,12 @@ hsize_t diff_attr(hid_t loc1_id, error: H5E_BEGIN_TRY { if(buf1) { - if(TRUE == H5Tdetect_class(mtype1_id, H5T_VLEN)) + if(TRUE == h5tools_detect_vlen(mtype1_id)) H5Dvlen_reclaim(mtype1_id, space1_id, H5P_DEFAULT, buf1); HDfree(buf1); } /* end if */ if(buf2) { - if(TRUE == H5Tdetect_class(mtype2_id, H5T_VLEN)) + if(TRUE == h5tools_detect_vlen(mtype2_id)) H5Dvlen_reclaim(mtype2_id, space2_id, H5P_DEFAULT, buf2); HDfree(buf2); } /* end if */ diff --git a/tools/lib/h5diff_dset.c b/tools/lib/h5diff_dset.c index f8fc493..230e7bb 100644 --- a/tools/lib/h5diff_dset.c +++ b/tools/lib/h5diff_dset.c @@ -213,8 +213,10 @@ hsize_t diff_datasetid( hid_t did1, void *buf2=NULL; void *sm_buf1=NULL; void *sm_buf2=NULL; + hid_t sm_space; /*stripmine data space */ size_t need; /* bytes needed for malloc */ int i; + unsigned int vl_data = 0; /*contains VL datatypes */ /* Get the dataspace handle */ if ( (sid1 = H5Dget_space(did1)) < 0 ) @@ -325,6 +327,11 @@ hsize_t diff_datasetid( hid_t did1, can_compare=0; options->not_cmp=1; } + + /* Check if type is either VLEN-data or VLEN-string to reclaim any + * VLEN memory buffer later */ + if( TRUE == h5tools_detect_vlen(m_tid1) ) + vl_data = TRUE; /*------------------------------------------------------------------------- * only attempt to compare if possible @@ -332,7 +339,6 @@ hsize_t diff_datasetid( hid_t did1, */ if(can_compare) /* it is possible to compare */ { - unsigned int vl_data = 0; /*contains VL datatypes */ /*------------------------------------------------------------------------- * get number of elements @@ -380,10 +386,6 @@ hsize_t diff_datasetid( hid_t did1, name2 = diff_basename(obj2_name); - /* check if we have VL data in the dataset's datatype */ - if(TRUE == H5Tdetect_class(m_tid1, H5T_VLEN)) - vl_data = TRUE; - /*------------------------------------------------------------------------- * read/compare *------------------------------------------------------------------------- @@ -417,13 +419,11 @@ hsize_t diff_datasetid( hid_t did1, hsize_t p_nelmts = nelmts1; /*total selected elmts */ hsize_t elmtno; /*counter */ int carry; /*counter carry value */ - unsigned int vl_data = 0; /*contains VL datatypes */ /* stripmine info */ hsize_t sm_size[H5S_MAX_RANK]; /*stripmine size */ hsize_t sm_nbytes; /*bytes per stripmine */ hsize_t sm_nelmts; /*elements per stripmine*/ - hid_t sm_space; /*stripmine data space */ /* hyperslab info */ hsize_t hs_offset[H5S_MAX_RANK]; /*starting offset */ @@ -558,40 +558,52 @@ hsize_t diff_datasetid( hid_t did1, error: options->err_stat=1; - /* free */ - if (buf1!=NULL) - { - free(buf1); - buf1=NULL; - } - if (buf2!=NULL) - { - free(buf2); - buf2=NULL; - } - if (sm_buf1!=NULL) - { - free(sm_buf1); - sm_buf1=NULL; - } - if (sm_buf2!=NULL) - { - free(sm_buf2); - sm_buf2=NULL; - } - - /* disable error reporting */ - H5E_BEGIN_TRY { - H5Sclose(sid1); - H5Sclose(sid2); - H5Tclose(f_tid1); - H5Tclose(f_tid2); - H5Tclose(m_tid1); - H5Tclose(m_tid2); - /* enable error reporting */ - } H5E_END_TRY; - - return nfound; + /* free */ + if (buf1!=NULL) + { + /* reclaim any VL memory, if necessary */ + if(vl_data) + H5Dvlen_reclaim(m_tid1, sid1, H5P_DEFAULT, buf1); + free(buf1); + buf1=NULL; + } + if (buf2!=NULL) + { + /* reclaim any VL memory, if necessary */ + if(vl_data) + H5Dvlen_reclaim(m_tid2, sid2, H5P_DEFAULT, buf2); + free(buf2); + buf2=NULL; + } + if (sm_buf1!=NULL) + { + /* reclaim any VL memory, if necessary */ + if(vl_data) + H5Dvlen_reclaim(m_tid1, sm_space, H5P_DEFAULT, sm_buf1); + free(sm_buf1); + sm_buf1=NULL; + } + if (sm_buf2!=NULL) + { + /* reclaim any VL memory, if necessary */ + if(vl_data) + H5Dvlen_reclaim(m_tid1, sm_space, H5P_DEFAULT, sm_buf2); + free(sm_buf2); + sm_buf2=NULL; + } + + /* disable error reporting */ + H5E_BEGIN_TRY { + H5Sclose(sid1); + H5Sclose(sid2); + H5Tclose(f_tid1); + H5Tclose(f_tid2); + H5Tclose(m_tid1); + H5Tclose(m_tid2); + /* enable error reporting */ + } H5E_END_TRY; + + return nfound; } /*------------------------------------------------------------------------- diff --git a/tools/lib/h5tools.c b/tools/lib/h5tools.c index 9790865..18d9035 100644 --- a/tools/lib/h5tools.c +++ b/tools/lib/h5tools.c @@ -627,11 +627,54 @@ h5tools_ncols(const char *s) } /*------------------------------------------------------------------------- + * Function: h5tools_detect_vlen + * + * Purpose: Recursive check for any variable length data in given type. + * + * Return: + * TRUE : type conatains any variable length data + * FALSE : type doesn't contain any variable length data + * Negative value: error occur + * + * Programmer: Jonathan Kim March 18, 2011 + *------------------------------------------------------------------------- + */ +htri_t +h5tools_detect_vlen(hid_t tid) +{ + htri_t status; + htri_t ret = FALSE; + /* recursive detect any vlen data values in type (compound, array ...) */ + status = H5Tdetect_class(tid, H5T_VLEN); + if ( (status == TRUE) || (status < 0) ) + { + ret = status; + goto done; + } + + /* recursive detect any vlen string in type (compound, array ...) */ + status = h5tools_detect_vlen_str(tid); + if ( (status == TRUE) || (status < 0) ) + + { + ret = status; + goto done; + } + +done: + return ret; +} + + +/*------------------------------------------------------------------------- * Function: h5tools_detect_vlen_str * * Purpose: Recursive check for variable length string of a datatype. * - * Return: TRUE if type conatains a variable string type, else FALSE + * Return: + * TRUE : type conatains any variable length string + * FALSE : type doesn't contain any variable length string + * Negative value: error occur * *------------------------------------------------------------------------- */ @@ -640,32 +683,55 @@ h5tools_detect_vlen_str(hid_t tid) { int i = 0; int n = 0; - htri_t has_vlen_str = FALSE; + htri_t ret = FALSE; H5T_class_t tclass = -1; + hid_t btid; + hid_t mtid; - if (H5Tis_variable_str(tid) == TRUE) - return TRUE; + ret = H5Tis_variable_str(tid); + if ( (ret == TRUE) || (ret < 0) ) + goto done; tclass = H5Tget_class(tid); - if (tclass == H5T_ARRAY) { - hid_t btid = H5Tget_super(tid); - has_vlen_str = h5tools_detect_vlen_str(btid); - H5Tclose(btid); - return has_vlen_str; + if (tclass == H5T_ARRAY) + { + btid = H5Tget_super(tid); + if (btid < 0) + { + ret = (htri_t) btid; + goto done; + } + ret = h5tools_detect_vlen_str(btid); + if ( (ret == TRUE) || (ret < 0) ) + { + H5Tclose(btid); + goto done; + } } - else if (tclass == H5T_COMPOUND) { + else if (tclass == H5T_COMPOUND) + { n = H5Tget_nmembers(tid); - for (i = 0; i < n; i++) { - hid_t mtid = H5Tget_member_type(tid, i); - has_vlen_str = h5tools_detect_vlen_str(mtid); - if (has_vlen_str == TRUE) { + if (n < 0) + { + n = ret; + goto done; + } + + for (i = 0; i < n; i++) + { + mtid = H5Tget_member_type(tid, i); + ret = h5tools_detect_vlen_str(mtid); + if ( (ret == TRUE) || (ret < 0) ) + { H5Tclose(mtid); - return TRUE; + goto done; } H5Tclose(mtid); } } - return FALSE; + +done: + return ret; } /*------------------------------------------------------------------------- diff --git a/tools/lib/h5tools.h b/tools/lib/h5tools.h index 66d5167..fcfebc0 100644 --- a/tools/lib/h5tools.h +++ b/tools/lib/h5tools.h @@ -559,6 +559,7 @@ H5TOOLS_DLL hid_t h5tools_get_native_type(hid_t type); H5TOOLS_DLL hid_t h5tools_get_little_endian_type(hid_t type); H5TOOLS_DLL hid_t h5tools_get_big_endian_type(hid_t type); +H5TOOLS_DLL htri_t h5tools_detect_vlen(hid_t tid); H5TOOLS_DLL htri_t h5tools_detect_vlen_str(hid_t tid); H5TOOLS_DLL void h5tools_dump_simple_data(FILE *stream, const h5tool_format_t *info, hid_t container, -- cgit v0.12