From e8d97aef7dfda3da6ceb599828fd641fa26cd8a9 Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Thu, 17 Mar 2011 18:00:14 -0500 Subject: [svn-r20266] Purpose: Fixed Bug 2216 - GMQS: h5diff - memory leak when compares vlen string in dataset or attributes Description: 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), tejeda (mac32-LE), linew (solaris-BE), Cmake - jam --- tools/lib/h5diff_attr.c | 16 +++++--- tools/lib/h5diff_dset.c | 97 ++++++++++++++++++++++++++++--------------------- 2 files changed, 67 insertions(+), 46 deletions(-) diff --git a/tools/lib/h5diff_attr.c b/tools/lib/h5diff_attr.c index 7e5fb04..1742028 100644 --- a/tools/lib/h5diff_attr.c +++ b/tools/lib/h5diff_attr.c @@ -221,12 +221,16 @@ 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 == H5Tdetect_class(mtype1_id, H5T_VLEN) || + TRUE == h5tools_detect_vlen_str(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 == H5Tdetect_class(mtype2_id, H5T_VLEN) || + TRUE == h5tools_detect_vlen_str(mtype2_id)) H5Dvlen_reclaim(mtype2_id, space2_id, H5P_DEFAULT, buf2); HDfree(buf2); buf2 = NULL; @@ -256,12 +260,14 @@ hsize_t diff_attr(hid_t loc1_id, error: H5E_BEGIN_TRY { if(buf1) { - if(TRUE == H5Tdetect_class(mtype1_id, H5T_VLEN)) + if(TRUE == H5Tdetect_class(mtype1_id, H5T_VLEN) || + TRUE == h5tools_detect_vlen_str(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 == H5Tdetect_class(mtype2_id, H5T_VLEN) || + TRUE == h5tools_detect_vlen_str(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..b21230b 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,14 @@ 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 == H5Tdetect_class(m_tid1, H5T_VLEN) || + TRUE == h5tools_detect_vlen_str(m_tid1)) + { + vl_data = TRUE; + } /*------------------------------------------------------------------------- * only attempt to compare if possible @@ -332,7 +342,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 +389,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 +422,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 +561,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; } /*------------------------------------------------------------------------- -- cgit v0.12