[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

4 files changed, 143 insertions, 62 deletions

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,