[svn-r20539] Description:

	Bring Coverity changes back to trunk:

r20276:
Add recursive is vlen string function.
Cleanup resource leaks for issues: 200,202,329,688,811,812


r20277:
Check types and close by adding error section: issue 687


r20278:
Replaced implicit pointer conversion with (ocrt_info.new_obj != NULL).


r20280:
Addressed coverity issues 927-929 & 583.  The real issue is failure to check 
file name length -- at least at the H5FD interface level.  This needs more work, but at least I have dealt with the issue in H5FDfamily.c


r20337:
H5O_type_t  obj_type = H5O_TYPE_UNKNOWN;


r20338:
Added udata.name = NULL; to prevent potential uninitialized use after done: label.


r20339:
coverity issues: 686,828,1670-1673,1707-1711


Tested on:
        FreeBSD/32 6.3 (duty) in debug mode
        FreeBSD/64 6.3 (liberty) w/C++ & FORTRAN, in debug mode
        FreeBSD/32 8.2 (loyalty) w/gcc4.6, in debug mode
        FreeBSD/64 8.2 (freedom) w/gcc4.6, in debug mode
        Linux/32 2.6 (jam) w/PGI compilers, w/default API=1.8.x,
                w/C++ & FORTRAN, w/threadsafe, in debug mode
        Linux/64-amd64 2.6 (amani) w/Intel compilers, w/default API=1.6.x,
                w/C++ & FORTRAN, in production mode
        Solaris/32 2.10 (linew) w/deprecated symbols disabled, w/C++ & FORTRAN,
                w/szip filter, w/threadsafe, in production mode
        Linux/PPC 2.6 (heiwa) w/C++ & FORTRAN, w/threadsafe, in debug mode
        Linux/64-amd64 2.6 (abe) w/parallel, w/FORTRAN, in debug mode

6 files changed, 214 insertions, 162 deletions

diff --git a/src/H5FDfamily.c b/src/H5FDfamily.c
index bd09ea4..5fd0294 100644
--- a/src/H5FDfamily.c
+++ b/src/H5FDfamily.c
@@ -628,7 +628,7 @@ H5FD_family_sb_encode(H5FD_t *_file, char *name/*out*/, unsigned char *buf/*out*
     FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5FD_family_sb_encode)
 
     /* Name and version number */
-    HDstrncpy(name, "NCSAfami", (size_t)8);
+    HDstrncpy(name, "NCSAfami", (size_t)9);
     name[8] = '\0';
 
     /* Store member file size.  Use the member file size from the property here.
@@ -690,9 +690,9 @@ H5FD_family_sb_decode(H5FD_t *_file, const char UNUSED *name, const unsigned cha
 
     /* Check if member size from file access property is correct */
     if(msize != file->pmem_size) {
-        char                err_msg[128];
+        char err_msg[128];
 
-        sprintf(err_msg, "Family member size should be %lu.  But the size from file access property is %lu", (unsigned long)msize, (unsigned long)file->pmem_size);
+        HDsnprintf(err_msg, sizeof(err_msg), "Family member size should be %lu.  But the size from file access property is %lu", (unsigned long)msize, (unsigned long)file->pmem_size);
         HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, err_msg)
     } /* end if */
 
@@ -806,14 +806,14 @@ H5FD_family_open(const char *name, unsigned flags, hid_t fapl_id,
     file->flags = flags;
 
     /* Check that names are unique */
-    sprintf(memb_name, name, 0);
-    sprintf(temp, name, 1);
+    HDsnprintf(memb_name, sizeof(memb_name), name, 0);
+    HDsnprintf(temp, sizeof(temp), name, 1);
     if(!HDstrcmp(memb_name, temp))
         HGOTO_ERROR(H5E_FILE, H5E_FILEEXISTS, NULL, "file names not unique")
 
     /* Open all the family members */
     while(1) {
-        sprintf(memb_name, name, file->nmembs);
+        HDsnprintf(memb_name, sizeof(memb_name), name, file->nmembs);
 
         /* Enlarge member array */
         if(file->nmembs >= file->amembs) {
@@ -1094,7 +1094,7 @@ H5FD_family_set_eoa(H5FD_t *_file, H5FD_mem_t type, haddr_t abs_eoa)
         /* Create another file if necessary */
         if(u >= file->nmembs || !file->memb[u]) {
             file->nmembs = MAX(file->nmembs, u+1);
-            sprintf(memb_name, file->name, u);
+            HDsnprintf(memb_name, sizeof(memb_name), file->name, u);
             H5E_BEGIN_TRY {
                 H5_CHECK_OVERFLOW(file->memb_size, hsize_t, haddr_t);
                 file->memb[u] = H5FDopen(memb_name, file->flags | H5F_ACC_CREAT,
diff --git a/src/H5Gnode.c b/src/H5Gnode.c
index 5a2d736..5f32eb5 100644
--- a/src/H5Gnode.c
+++ b/src/H5Gnode.c
@@ -1239,7 +1239,7 @@ H5G_node_copy(H5F_t *f, hid_t dxpl_id, const void UNUSED *_lt_key, haddr_t addr,
         H5O_link_t  lnk;                /* Link to insert */
         const char  *name;              /* Name of source object */
         H5G_entry_t tmp_src_ent;        /* Temperary copy. Change will not affect the cache */
-        H5O_type_t  obj_type;           /* Target object type */
+        H5O_type_t  obj_type = H5O_TYPE_UNKNOWN; /* Target object type */
         H5G_copy_file_ud_t *cpy_udata;  /* Copy file udata */
         H5G_obj_create_t gcrt_info;     /* Group creation info */
 
diff --git a/src/H5Gstab.c b/src/H5Gstab.c
index 7d0ad48..afa137c 100644
--- a/src/H5Gstab.c
+++ b/src/H5Gstab.c
@@ -711,6 +711,7 @@ H5G_stab_get_name_by_idx(H5O_loc_t *oloc, H5_iter_order_t order, hsize_t n,
     H5HL_t *heap = NULL;        /* Pointer to local heap */
     H5O_stab_t	stab;	        /* Info about local heap & B-tree */
     H5G_bt_it_gnbi_t udata;     /* Iteration information */
+    hbool_t udata_valid = FALSE;        /* Whether iteration information is valid */
     ssize_t ret_value;          /* Return value */
 
     FUNC_ENTER_NOAPI(H5G_stab_get_name_by_idx, FAIL)
@@ -744,6 +745,7 @@ H5G_stab_get_name_by_idx(H5O_loc_t *oloc, H5_iter_order_t order, hsize_t n,
     udata.common.op = H5G_stab_get_name_by_idx_cb;
     udata.heap = heap;
     udata.name = NULL;
+    udata_valid = TRUE;
 
     /* Iterate over the group members */
     if(H5B_iterate(oloc->file, dxpl_id, H5B_SNODE, stab.btree_addr, H5G_node_by_idx, &udata) < 0)
@@ -769,7 +771,7 @@ done:
         HDONE_ERROR(H5E_SYM, H5E_PROTECT, FAIL, "unable to unprotect symbol table heap")
 
     /* Free the duplicated name */
-    if(udata.name != NULL)
+    if(udata_valid && udata.name != NULL)
         H5MM_xfree(udata.name);
 
     FUNC_LEAVE_NOAPI(ret_value)
diff --git a/src/H5Tcommit.c b/src/H5Tcommit.c
index e109f95..d3779e5 100644
--- a/src/H5Tcommit.c
+++ b/src/H5Tcommit.c
@@ -221,7 +221,7 @@ done:
     /* If the datatype was committed but something failed after that, we need
      * to return it to the state it was in before it was committed.
      */
-    if(ret_value < 0 && ocrt_info.new_obj) {
+    if(ret_value < 0 && (NULL != ocrt_info.new_obj)) {
 	if(dt->shared->state == H5T_STATE_OPEN && dt->sh_loc.type == H5O_SHARE_TYPE_COMMITTED) {
             /* Remove the datatype from the list of opened objects in the file */
             if(H5FO_top_decr(dt->sh_loc.file, dt->sh_loc.u.loc.oh_addr) < 0)
diff --git a/tools/h5dump/h5dump.c b/tools/h5dump/h5dump.c
index f479774..7a3663f 100644
--- a/tools/h5dump/h5dump.c
+++ b/tools/h5dump/h5dump.c
@@ -1453,13 +1453,16 @@ dump_attr_cb(hid_t oid, const char *attr_name, const H5A_info_t UNUSED *info, vo
 static herr_t
 dump_selected_attr(hid_t loc_id, const char *name)
 {
-    hid_t  oid, attr_id;
+    hid_t  oid = -1;
+    hid_t  attr_id = -1;
     char *obj_name;
     const char *attr_name;
     int j;
 
     j = (int)HDstrlen(name) - 1;
     obj_name = (char *)HDmalloc((size_t)j + 2);
+    if(obj_name == NULL)
+        goto error;
 
     /* find the last / */
     while(j >= 0) {
@@ -1485,8 +1488,7 @@ dump_selected_attr(hid_t loc_id, const char *name)
         indentation(COL);
         error_msg("unable to open object \"%s\"\n", obj_name);
         end_obj(dump_header_format->attributeend, dump_header_format->attributeblockend);
-        h5tools_setstatus(EXIT_FAILURE);
-        return FAIL;
+        goto error;
     } /* end if */
 
     if((attr_id = H5Aopen(oid, attr_name, H5P_DEFAULT)) >= 0) {
@@ -1512,17 +1514,27 @@ dump_selected_attr(hid_t loc_id, const char *name)
         indentation(COL);
         error_msg("unable to open attribute \"%s\"\n", obj_name);
         end_obj(dump_header_format->attributeend, dump_header_format->attributeblockend);
-        h5tools_setstatus(EXIT_FAILURE);
+        goto error;
     }
 
     /* Close object */
     if(H5Oclose(oid) < 0) {
-        h5tools_setstatus(EXIT_FAILURE);
-        return FAIL;
+        goto error;
     } /* end if */
 
     HDfree(obj_name);
     return SUCCEED;
+    
+error:
+    h5tools_setstatus(EXIT_FAILURE);
+    if(obj_name)
+        HDfree(obj_name);
+    
+    H5E_BEGIN_TRY {
+        H5Oclose(oid);
+        H5Aclose(attr_id);
+    } H5E_END_TRY;
+    return FAIL;
 }
 
 /*-------------------------------------------------------------------------
@@ -1555,7 +1567,11 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
 
     /* Build the object's path name */
     obj_path = (char *)HDmalloc(HDstrlen(prefix) + HDstrlen(name) + 2);
-    HDassert(obj_path);
+    if(!obj_path) {
+        ret = FAIL;
+        goto done;
+    } 
+    
     HDstrcpy(obj_path, prefix);
     HDstrcat(obj_path, "/");
     HDstrcat(obj_path, name);
@@ -1571,17 +1587,14 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
             goto done;
         } /* end if */
 
-        switch(oinfo.type)
-        {
+        switch(oinfo.type) {
         case H5O_TYPE_GROUP:
-            if((obj = H5Gopen2(group, name, H5P_DEFAULT)) < 0)
-            {
+            if((obj = H5Gopen2(group, name, H5P_DEFAULT)) < 0)  {
                 error_msg("unable to dump group \"%s\"\n", name);
                 h5tools_setstatus(EXIT_FAILURE);
                 ret = FAIL;
             }
-            else
-            {
+            else {
                 char *old_prefix; /* Pointer to previous prefix */
 
                 /* Keep copy of prefix before iterating into group */
@@ -1624,7 +1637,8 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
                             ret = FAIL;
                             H5Dclose(obj);
                             goto done;
-                        } else if(found_obj->displayed) {
+                        } 
+                        else if(found_obj->displayed) {
                             indentation(indent);
 
                             if(!doxml) {
@@ -1635,7 +1649,8 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
                                 indentation(indent);
                                 end_obj(dump_header_format->datasetend,
                                         dump_header_format->datasetblockend);
-                            } else {
+                            } 
+                            else {
                                 /* the XML version */
                                 char *t_obj_path = xml_escape_the_name(obj_path);
                                 char *t_prefix = xml_escape_the_name(HDstrcmp(prefix,"") ? prefix : "/");
@@ -1675,14 +1690,16 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
 
                             H5Dclose(obj);
                             goto done;
-                        } else {
+                        } 
+                        else {
                             found_obj->displayed = TRUE;
                         }
                     } /* end if */
 
                     dump_function_table->dump_dataset_function(obj, name, NULL);
                     H5Dclose(obj);
-                } else {
+                } 
+                else {
                     error_msg("unable to dump dataset \"%s\"\n", name);
                     h5tools_setstatus(EXIT_FAILURE);
                     ret = FAIL;
@@ -1694,7 +1711,8 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
                     error_msg("unable to dump datatype \"%s\"\n", name);
                     h5tools_setstatus(EXIT_FAILURE);
                     ret = FAIL;
-                } else {
+                } 
+                else {
                     dump_function_table->dump_named_datatype_function(obj, name);
                     H5Tclose(obj);
                 }
@@ -1725,12 +1743,14 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
                     error_msg("unable to get link value\n");
                     h5tools_setstatus(EXIT_FAILURE);
                     ret = FAIL;
-                } else {
+                } 
+                else {
                     /* print the value of a soft link */
                     if (!doxml) {
                         /* Standard DDL: no modification */
                         printf("LINKTARGET \"%s\"\n", targbuf);
-                    } else {
+                    } 
+                    else {
                         /* XML */
                         char linkxid[100];
                         char parentxid[100];
@@ -1771,7 +1791,8 @@ dump_all_cb(hid_t group, const char *name, const H5L_info_t *linfo, void UNUSED
                                     targetxid,      /* TargetObj */
                                     parentxid,      /* Parents */
                                     t_prefix);      /* H5ParentPaths */
-                        } else {
+                        } 
+                        else {
                             /* dangling link -- omit from xml attributes */
                             printf("<%sSoftLink LinkName=\"%s\" "
                                    "OBJ-XID=\"%s\" "
@@ -3605,35 +3626,40 @@ handle_datasets(hid_t fid, const char *dset, void *data, int pe, const char *dis
     if(sset) {
         unsigned int i;
         hid_t sid = H5Dget_space(dsetid);
-        unsigned int ndims = H5Sget_simple_extent_ndims(sid);
+        int ndims = H5Sget_simple_extent_ndims(sid);
 
         H5Sclose(sid);
+        if(ndims < 0) {
+            error_msg("H5Sget_simple_extent_ndims failed\n");
+            h5tools_setstatus(EXIT_FAILURE);
+            return;
+        }
 
         if(!sset->start.data || !sset->stride.data || !sset->count.data || !sset->block.data) {
             /* they didn't specify a ``stride'' or ``block''. default to 1 in all
              * dimensions */
             if(!sset->start.data) {
                 /* default to (0, 0, ...) for the start coord */
-                sset->start.data = (hsize_t *)calloc(ndims, sizeof(hsize_t));
+                sset->start.data = (hsize_t *)calloc((size_t)ndims, sizeof(hsize_t));
                 sset->start.len = ndims;
             }
 
             if(!sset->stride.data) {
-                sset->stride.data = (hsize_t *)calloc(ndims, sizeof(hsize_t));
+                sset->stride.data = (hsize_t *)calloc((size_t)ndims, sizeof(hsize_t));
                 sset->stride.len = ndims;
                 for (i = 0; i < ndims; i++)
                     sset->stride.data[i] = 1;
             }
 
             if(!sset->count.data) {
-                sset->count.data = (hsize_t *)calloc(ndims, sizeof(hsize_t));
+                sset->count.data = (hsize_t *)calloc((size_t)ndims, sizeof(hsize_t));
                 sset->count.len = ndims;
                 for (i = 0; i < ndims; i++)
                     sset->count.data[i] = 1;
             }
 
             if(!sset->block.data) {
-                sset->block.data = (hsize_t *)calloc(ndims, sizeof(hsize_t));
+                sset->block.data = (hsize_t *)calloc((size_t)ndims, sizeof(hsize_t));
                 sset->block.len = ndims;
                 for (i = 0; i < ndims; i++)
                     sset->block.data[i] = 1;
@@ -4222,10 +4248,10 @@ parse_start:
              */
             do {
                 switch ((char)opt) {
-                case 's': free(s->start.data); parse_hsize_list(opt_arg, &s->start); break;
-                case 'S': free(s->stride.data); parse_hsize_list(opt_arg, &s->stride); break;
-                case 'c': free(s->count.data); parse_hsize_list(opt_arg, &s->count); break;
-                case 'k': free(s->block.data); parse_hsize_list(opt_arg, &s->block); break;
+                case 's': if(s->start.data) free(s->start.data); parse_hsize_list(opt_arg, &s->start); break;
+                case 'S': if(s->stride.data) free(s->stride.data); parse_hsize_list(opt_arg, &s->stride); break;
+                case 'c': if(s->count.data) free(s->count.data); parse_hsize_list(opt_arg, &s->count); break;
+                case 'k': if(s->block.data) free(s->block.data); parse_hsize_list(opt_arg, &s->block); break;
                 default: goto end_collect;
                 }
             } while ((opt = get_option(argc, argv, s_opts, l_opts)) != EOF);
@@ -5146,7 +5172,9 @@ xml_print_datatype(hid_t type, unsigned in_group)
                 printf("<%sAtomicType>\n",xmlnsprefix);
                 indent += COL;
                 indentation(indent);
-                printf("<%sOpaqueType Tag=\"%s\" ",xmlnsprefix, H5Tget_tag(type));
+                mname = H5Tget_tag(type);
+                printf("<%sOpaqueType Tag=\"%s\" ",xmlnsprefix, mname);
+                free(mname);
                 size = H5Tget_size(type);
                 printf("Size=\"%lu\"/>\n", (unsigned long)size);
                 indent -= COL;
@@ -6095,67 +6123,68 @@ xml_dump_group(hid_t gid, const char *name)
 static int
 xml_print_refs(hid_t did, int source)
 {
-    herr_t                  e;
-    hid_t                   type, space;
-    char                   *buf;
-    hobj_ref_t             *refbuf;
-    hsize_t                 ssiz;
-    hsize_t                 i;
+    herr_t e;
+    hid_t type, space;
+    char *buf = NULL;
+    hobj_ref_t *refbuf = NULL;
+    hssize_t ssiz;
+    hsize_t i;
+    size_t tsiz;
 
     if (source == DATASET_DATA) {
         type = H5Dget_type(did);
-    } else if (source == ATTRIBUTE_DATA) {
+    }
+    else if (source == ATTRIBUTE_DATA) {
         type = H5Aget_type(did);
-    } else {
+    }
+    else {
         /* return an error */
         return FAIL;
     }
     if (H5Tget_class(type) != H5T_REFERENCE) {
         /* return an error */
-        return FAIL;
+        goto error;
     }
     if (!H5Tequal(type, H5T_STD_REF_OBJ)) {
         /* region ref not supported yet... */
         /* return an error */
-        return FAIL;
+        goto error;
     }
     if (source == DATASET_DATA) {
         space = H5Dget_space(did);
-        ssiz = H5Sget_simple_extent_npoints(space);
-        ssiz *= H5Tget_size(type);
-
-        buf = (char *)calloc((size_t)ssiz, sizeof(char));
-        if(buf == NULL)
-            return FAIL;
+        if ((ssiz = H5Sget_simple_extent_npoints(space)) < 0)
+            goto error;
+        if ((tsiz = H5Tget_size(type)) == 0)
+            goto error;
+
+        buf = (char *) calloc((size_t)(ssiz * tsiz), sizeof(char));
+        if (buf == NULL)
+            goto error;
         e = H5Dread(did, H5T_STD_REF_OBJ, H5S_ALL, H5S_ALL, H5P_DEFAULT, buf);
         /* need to check result here */
-        if(e < 0) {
-            free(buf);
-            return FAIL;
+        if (e < 0) {
+            goto error;
         }
-    } else if (source == ATTRIBUTE_DATA) {
+    }
+    else if (source == ATTRIBUTE_DATA) {
         space = H5Aget_space(did);
-        ssiz = H5Sget_simple_extent_npoints(space);
-        ssiz *= H5Tget_size(type);
+        if ((ssiz = H5Sget_simple_extent_npoints(space)) < 0)
+            goto error;
+        if ((tsiz = H5Tget_size(type)) == 0)
+            goto error;
 
-        buf = (char *)calloc((size_t)ssiz, sizeof(char));
+        buf = (char *) calloc((size_t)(ssiz * tsiz), sizeof(char));
         if (buf == NULL) {
-            free(buf);
-            return FAIL;
+            goto error;
         }
         e = H5Aread(did, H5T_STD_REF_OBJ, buf);
         /* need to check the result here */
-        if(e < 0) {
-            free(buf);
-            return FAIL;
+        if (e < 0) {
+            goto error;
         }
-    } else {
-        /* error */
-        return FAIL;
     }
 
     refbuf = (hobj_ref_t *) buf;
-    ssiz = H5Sget_simple_extent_npoints(space);
 
     for (i = 0; i < ssiz; i++) {
         const char *path;
@@ -6165,7 +6194,8 @@ xml_print_refs(hid_t did, int source)
 
         if (!path) {
             printf("\"%s\"\n", "NULL");
-        } else {
+        }
+        else {
             char *t_path = xml_escape_the_string(path, -1);
 
             printf("\"%s\"\n", t_path);
@@ -6176,8 +6206,19 @@ xml_print_refs(hid_t did, int source)
     }
 
     free(buf);
-
+    H5Tclose(type);
+    H5Sclose(space);
     return SUCCEED;
+    
+error:
+    if(buf)
+        free(buf);
+
+    H5E_BEGIN_TRY {
+        H5Tclose(type);
+        H5Sclose(space);
+    } H5E_END_TRY;
+    return FAIL;
 }
 
 /*-------------------------------------------------------------------------
@@ -6196,77 +6237,78 @@ xml_print_refs(hid_t did, int source)
 static int
 xml_print_strs(hid_t did, int source)
 {
-    herr_t                  e;
-    hid_t                   type, space;
-    void                   *buf;
-    char                   *bp;
-    char                   *onestring=NULL;
-    hsize_t                 ssiz;
-    size_t                  tsiz, str_size=0;
-    size_t                  i;
-    htri_t                  is_vlstr;
+    herr_t e;
+    hid_t type, space;
+    void *buf = NULL;
+    char *bp;
+    char *onestring = NULL;
+    hssize_t ssiz;
+    size_t tsiz;
+    size_t str_size = 0;
+    size_t i;
+    htri_t is_vlstr;
 
     if (source == DATASET_DATA) {
         type = H5Dget_type(did);
-    } else if (source == ATTRIBUTE_DATA) {
+    }
+    else if (source == ATTRIBUTE_DATA) {
         type = H5Aget_type(did);
-    } else {
+    }
+    else {
         /* return an error */
         return FAIL;
     }
     if (H5Tget_class(type) != H5T_STRING) {
         /* return an error */
-        return FAIL;
+        goto error;
     }
-    is_vlstr = H5Tis_variable_str(type);
+    /* Check if we have VL data in the dataset's datatype */
+    is_vlstr = (TRUE == H5Tis_variable_str(type));
 
     if (source == DATASET_DATA) {
         space = H5Dget_space(did);
-        ssiz = H5Sget_simple_extent_npoints(space);
-        ssiz *= H5Tget_size(type);
+        if((ssiz = H5Sget_simple_extent_npoints(space)) < 0)
+            goto error;
+        if((tsiz = H5Tget_size(type)) == 0)
+            goto error;
 
-        buf = malloc((size_t)ssiz);
-        if(buf == NULL)
-            return FAIL;
+        buf = malloc((size_t)(ssiz * tsiz));
+        if (buf == NULL)
+            goto error;
 
         e = H5Dread(did, type, H5S_ALL, H5S_ALL, H5P_DEFAULT, buf);
         if (e < 0) {
-            free(buf);
-            return FAIL;
+            goto error;
         }
-    } else if (source == ATTRIBUTE_DATA) {
+    }
+    else if (source == ATTRIBUTE_DATA) {
         space = H5Aget_space(did);
-        ssiz = H5Sget_simple_extent_npoints(space);
-        ssiz *= H5Tget_size(type);
+        if((ssiz = H5Sget_simple_extent_npoints(space)) < 0)
+            goto error;
+        if((tsiz = H5Tget_size(type)) == 0)
+            goto error;
 
-        buf = malloc((size_t)ssiz);
-        if(buf == NULL)
-            return FAIL;
+        buf = malloc((size_t)(ssiz * tsiz));
+        if (buf == NULL)
+            goto error;
 
         e = H5Aread(did, type, buf);
         if (e < 0) {
-            free(buf);
-            return FAIL;
+            goto error;
         }
-    } else {
-        /* error */
-        return FAIL;
     }
 
-    /* pull out each string... */
-    ssiz = H5Sget_simple_extent_npoints(space);
-
-    tsiz = H5Tget_size(type);
-    bp = (char*)buf;
-    if(!is_vlstr)
-        onestring = (char *)calloc(tsiz, sizeof(char));
+    bp = (char*) buf;
+    if (!is_vlstr)
+        onestring = (char *) calloc(tsiz, sizeof(char));
 
     for (i = 0; i < ssiz; i++) {
-        if(is_vlstr) {
-            onestring = *(char **)bp;
-            if(onestring)
-                str_size = (size_t)HDstrlen(onestring);
-        } else {
+        if (is_vlstr) {
+            onestring = *(char **) bp;
+            if (onestring)
+                str_size = (size_t) HDstrlen(onestring);
+        }
+        else {
             HDstrncpy(onestring, bp, tsiz);
             str_size = tsiz;
         }
@@ -6274,17 +6316,40 @@ xml_print_strs(hid_t did, int source)
 
         if (!onestring) {
             printf("NULL\n");
-        } else {
-            char *t_onestring = xml_escape_the_string(onestring, (int)str_size);
-
-            printf("\"%s\"\n", t_onestring);
-            free(t_onestring);
+        }
+        else {
+            char *t_onestring = xml_escape_the_string(onestring, (int) str_size);
+            if (t_onestring) {
+                printf("\"%s\"\n", t_onestring);
+                free(t_onestring);
+            }
         }
 
-       bp += tsiz;
+        bp += tsiz;
     }
 
+    /* Reclaim any VL memory, if necessary */
+    if (!is_vlstr)
+        if (onestring)
+            free(onestring);
+    if (buf) {
+        if (is_vlstr)
+            H5Dvlen_reclaim(type, space, H5P_DEFAULT, buf);
+        free(buf);
+    }
+    H5Tclose(type);
+    H5Sclose(space);
     return SUCCEED;
+    
+error:
+    if(buf)
+        free(buf);
+
+    H5E_BEGIN_TRY {
+        H5Tclose(type);
+        H5Sclose(space);
+    } H5E_END_TRY;
+    return FAIL;
 }
 
 /*-------------------------------------------------------------------------
diff --git a/tools/lib/h5tools.c b/tools/lib/h5tools.c
index 93a7390..5c8631e 100644
--- a/tools/lib/h5tools.c
+++ b/tools/lib/h5tools.c
@@ -637,30 +637,22 @@ h5tools_ncols(const char *s)
 htri_t
 h5tools_detect_vlen(hid_t tid)
 {
-    htri_t status;
-    htri_t ret = FALSE;
+    htri_t ret;
+
     /* recursive detect any vlen data values in type (compound, array ...) */
-    status = H5Tdetect_class(tid, H5T_VLEN);
-    if ( (status == TRUE) || (status < 0) )
-    {
-        ret = status;
+    ret = H5Tdetect_class(tid, H5T_VLEN);
+    if((ret == TRUE) || (ret < 0))
         goto done;
-    }
 
     /* recursive detect any vlen string in type (compound, array ...) */
-    status = h5tools_detect_vlen_str(tid);
-    if ( (status == TRUE) || (status < 0) )
-
-    {
-        ret = status;
+    ret = h5tools_detect_vlen_str(tid);
+    if((ret == TRUE) || (ret < 0))
         goto done;
-    }
 
 done:
     return ret;
 }
 
-
 /*-------------------------------------------------------------------------
  * Function: h5tools_detect_vlen_str
  *
@@ -676,48 +668,41 @@ done:
 htri_t
 h5tools_detect_vlen_str(hid_t tid)
 {
-    int i = 0;
-    int n = 0;
-    htri_t ret = FALSE;
     H5T_class_t tclass = -1;
-    hid_t btid;
-    hid_t mtid;
+    htri_t ret = FALSE;
 
     ret = H5Tis_variable_str(tid);
-    if ( (ret == TRUE) || (ret < 0) )
+    if((ret == TRUE) || (ret < 0))
         goto done;
 
     tclass = H5Tget_class(tid);
-    if (tclass == H5T_ARRAY) 
-    {
-        btid = H5Tget_super(tid);
-        if (btid < 0)
-        {
-            ret = (htri_t) btid;
+    if(tclass == H5T_ARRAY || tclass == H5T_VLEN) {
+        hid_t btid = H5Tget_super(tid);
+
+        if(btid < 0) {
+            ret = (htri_t)btid;
             goto done;
         }
         ret = h5tools_detect_vlen_str(btid);
-        if ( (ret == TRUE) || (ret < 0) ) 
-        {
+        if((ret == TRUE) || (ret < 0)) {
             H5Tclose(btid);
             goto done;
         }
     }
-    else if (tclass == H5T_COMPOUND) 
-    {
-        n = H5Tget_nmembers(tid);
-        if (n < 0) 
-        {
+    else if(tclass == H5T_COMPOUND) {
+        int i = 0;
+        int n = H5Tget_nmembers(tid);
+
+        if(n < 0) {
             n = ret;
             goto done;
         }
 
-        for (i = 0; i < n; i++) 
-        {
-            mtid = H5Tget_member_type(tid, i);
+        for(i = 0; i < n; i++) {
+            hid_t mtid = H5Tget_member_type(tid, i);
+
             ret = h5tools_detect_vlen_str(mtid);
-            if ( (ret == TRUE) || (ret < 0) ) 
-            {
+            if((ret == TRUE) || (ret < 0)) {
                 H5Tclose(mtid);
                 goto done;
             }