From 602d5ce44fa5a14bee73fe24128554a6130b9e97 Mon Sep 17 00:00:00 2001 From: Allen Byrne Date: Thu, 5 Mar 2009 13:29:23 -0500 Subject: [svn-r16549] Bug #608: Memory leak in H5DSset_label. Added code to free sub string ptr's belonging to buffer in H5DSset_label and H5DSget_label. Also added free of buffers in error section of both functions. Potential memory leaks may exist elsewhere, and this will not close the bug. Tested: h5committest vista 32 VS2008 --- hl/src/H5DS.c | 483 +++++++++++++++++++++++++++++++--------------------------- 1 file changed, 258 insertions(+), 225 deletions(-) diff --git a/hl/src/H5DS.c b/hl/src/H5DS.c index 46eedc4..f0c83f8 100644 --- a/hl/src/H5DS.c +++ b/hl/src/H5DS.c @@ -1385,151 +1385,171 @@ out: *------------------------------------------------------------------------- */ -herr_t H5DSset_label(hid_t did, - unsigned int idx, - const char *label) -{ - int has_labels; - hid_t sid; /* space ID */ - hid_t tid = -1; /* attribute type ID */ - hid_t aid = -1; /* attribute ID */ - int rank; /* rank of dataset */ - hsize_t dims[1]; /* dimensions of dataset */ - const char **buf=NULL; /* buffer to store in the attribute */ - H5I_type_t it; /* ID type */ - unsigned int i; - -/*------------------------------------------------------------------------- - * parameter checking - *------------------------------------------------------------------------- - */ - /* get ID type */ - if ((it = H5Iget_type(did)) < 0) - return FAIL; - - if (H5I_DATASET!=it) - return FAIL; - -/*------------------------------------------------------------------------- - * attribute "DIMENSION_LABELS" - *------------------------------------------------------------------------- - */ - - /* try to find the attribute "DIMENSION_LABELS" on the >>data<< dataset */ - if ((has_labels = H5LT_find_attribute(did,DIMENSION_LABELS)) < 0) - return FAIL; - - /* get dataset space */ - if ((sid = H5Dget_space(did)) < 0) - return FAIL; - - /* get rank */ - if ((rank=H5Sget_simple_extent_ndims(sid)) < 0) - goto out; - - /* close dataset space */ - if (H5Sclose(sid) < 0) - goto out; - -/*------------------------------------------------------------------------- - * make the attribute and insert label - *------------------------------------------------------------------------- - */ - - if (has_labels == 0) - { - dims[0] = rank; - - /* space for the attribute */ - if((sid = H5Screate_simple(1, dims, NULL)) < 0) - goto out; - - /* create the datatype */ - if((tid = H5Tcopy(H5T_C_S1)) < 0) - goto out; - if(H5Tset_size(tid, H5T_VARIABLE) < 0) - goto out; - - /* create the attribute */ - if((aid = H5Acreate2(did, DIMENSION_LABELS, tid, sid, H5P_DEFAULT, H5P_DEFAULT)) < 0) - goto out; - - /* allocate and initialize */ - buf = (const char **)malloc((size_t)rank * sizeof(char *)); - - if(buf == NULL) - goto out; - - for(i = 0; i < (unsigned int)rank; i++) - buf[i] = NULL; +herr_t H5DSset_label(hid_t did, unsigned int idx, const char *label) { + int has_labels; + hid_t sid; /* space ID */ + hid_t tid = -1; /* attribute type ID */ + hid_t aid = -1; /* attribute ID */ + int rank; /* rank of dataset */ + hsize_t dims[1]; /* dimensions of dataset */ + const char **buf = NULL; /* buffer to store in the attribute */ + H5I_type_t it; /* ID type */ + unsigned int i; + + /*------------------------------------------------------------------------- + * parameter checking + *------------------------------------------------------------------------- + */ + /* get ID type */ + if ((it = H5Iget_type(did)) < 0) + return FAIL; + + if (H5I_DATASET != it) + return FAIL; + + /*------------------------------------------------------------------------- + * attribute "DIMENSION_LABELS" + *------------------------------------------------------------------------- + */ + + /* try to find the attribute "DIMENSION_LABELS" on the >>data<< dataset */ + if ((has_labels = H5LT_find_attribute(did, DIMENSION_LABELS)) < 0) + return FAIL; + + /* get dataset space */ + if ((sid = H5Dget_space(did)) < 0) + return FAIL; + + /* get rank */ + if ((rank = H5Sget_simple_extent_ndims(sid)) < 0) + goto out; + + /* close dataset space */ + if (H5Sclose(sid) < 0) + goto out; + + /*------------------------------------------------------------------------- + * make the attribute and insert label + *------------------------------------------------------------------------- + */ + + if (has_labels == 0) { + dims[0] = rank; + + /* space for the attribute */ + if ((sid = H5Screate_simple(1, dims, NULL)) < 0) + goto out; + + /* create the datatype */ + if ((tid = H5Tcopy(H5T_C_S1)) < 0) + goto out; + if (H5Tset_size(tid, H5T_VARIABLE) < 0) + goto out; + + /* create the attribute */ + if ((aid = H5Acreate2(did, DIMENSION_LABELS, tid, sid, + H5P_DEFAULT, H5P_DEFAULT)) < 0) + goto out; + + /* allocate and initialize */ + buf = (const char **) malloc((size_t) rank * sizeof(char *)); + + if (buf == NULL) + goto out; + + for (i = 0; i < (unsigned int) rank; i++) + buf[i] = NULL; + + /* store the label information in the required index */ + buf[idx] = label; + + /* write the attribute with the label */ + if (H5Awrite(aid, tid, buf) < 0) + goto out; + + /* close */ + if (H5Sclose(sid) < 0) + goto out; + if (H5Tclose(tid) < 0) + goto out; + if (H5Aclose(aid) < 0) + goto out; + if (buf) + free((void *) buf); + } - /* store the label information in the required index */ - buf[idx] = label; + /*------------------------------------------------------------------------- + * just insert label + *------------------------------------------------------------------------- + */ - /* write the attribute with the label */ - if (H5Awrite(aid,tid,buf) < 0) - goto out; + else { + if ((aid = H5Aopen(did, DIMENSION_LABELS, H5P_DEFAULT)) < 0) + goto out; - /* close */ - if (H5Sclose(sid) < 0) - goto out; - if(H5Tclose(tid) < 0) - goto out; - if(H5Aclose(aid) < 0) - goto out; - if(buf) - free((void *)buf); - } + if ((tid = H5Aget_type(aid)) < 0) + goto out; -/*------------------------------------------------------------------------- - * just insert label - *------------------------------------------------------------------------- - */ + /* allocate and initialize */ + buf = (const char **) malloc((size_t) rank * sizeof(char *)); - else - { - if((aid = H5Aopen(did, DIMENSION_LABELS, H5P_DEFAULT)) < 0) - goto out; + if (buf == NULL) + goto out; - if((tid = H5Aget_type(aid)) < 0) - goto out; + /* read */ + if (H5Aread(aid, tid, (void *) buf) < 0) + goto out; - /* allocate and initialize */ - buf = (const char **)malloc((size_t)rank * sizeof(char *)); + /* free the ptr that will be replaced by label */ + if (buf[idx]) + free(buf[idx]); - if(buf == NULL) - goto out; + /* store the label information in the required index */ + buf[idx] = label; - /* read */ - if(H5Aread(aid, tid, (void *)buf) < 0) - goto out; + /* write the attribute with the new references */ + if (H5Awrite(aid, tid, buf) < 0) + goto out; - /* store the label information in the required index */ - buf[idx] = label; + /* label was brought in, so don't free */ + buf[idx] = NULL; - /* write the attribute with the new references */ - if (H5Awrite(aid,tid,buf) < 0) - goto out; + /* free all the ptr's from the H5Aread() */ + for (i = 0; i < (unsigned int) rank; i++) { + if (buf[i]) + free(buf[i]); + } - /* close */ - if (H5Tclose(tid) < 0) - goto out; - if (H5Aclose(aid) < 0) - goto out; - if (buf) - free((void *)buf); - } - - return SUCCEED; + /* close */ + if (H5Tclose(tid) < 0) + goto out; + if (H5Aclose(aid) < 0) + goto out; + if (buf) + free((void *) buf); + } - /* error zone, gracefully close */ -out: - H5E_BEGIN_TRY { - H5Sclose(sid); - H5Aclose(aid); - H5Tclose(tid); - } H5E_END_TRY; - return FAIL; + return SUCCEED; + + /* error zone, gracefully close */ + out: + if (buf) { + if (buf[idx]) /* check if we errored during H5Awrite */ + buf[idx] = NULL; /* don't free label */ + /* free all the ptr's from the H5Aread() */ + for (i = 0; i < (unsigned int) rank; i++) { + if (buf[i]) + free(buf[i]); + } + free(buf); + } + H5E_BEGIN_TRY + { + H5Sclose(sid); + H5Aclose(aid); + H5Tclose(tid); + }H5E_END_TRY; + return FAIL; } /*------------------------------------------------------------------------- @@ -1553,115 +1573,128 @@ out: * *------------------------------------------------------------------------- */ -ssize_t H5DSget_label(hid_t did, - unsigned int idx, - char *label, - size_t size) -{ - int has_labels; - hid_t sid; /* space ID */ - hid_t tid = -1; /* attribute type ID */ - hid_t aid = -1; /* attribute ID */ - int rank; /* rank of dataset */ - char **buf=NULL; /* buffer to store in the attribute */ - H5I_type_t it; /* ID type */ - size_t nbytes; - size_t copy_len; - -/*------------------------------------------------------------------------- - * parameter checking - *------------------------------------------------------------------------- - */ - /* get ID type */ - if ((it = H5Iget_type(did)) < 0) - return FAIL; - - if (H5I_DATASET!=it) - return FAIL; - -/*------------------------------------------------------------------------- - * attribute "DIMENSION_LABELS" - *------------------------------------------------------------------------- +ssize_t H5DSget_label(hid_t did, unsigned int idx, char *label, size_t size) { + int has_labels; + hid_t sid; /* space ID */ + hid_t tid = -1; /* attribute type ID */ + hid_t aid = -1; /* attribute ID */ + int rank; /* rank of dataset */ + char **buf = NULL; /* buffer to store in the attribute */ + H5I_type_t it; /* ID type */ + size_t nbytes; + size_t copy_len; + int i; + + /*------------------------------------------------------------------------- + * parameter checking + *------------------------------------------------------------------------- + */ + /* get ID type */ + if ((it = H5Iget_type(did)) < 0) + return FAIL; + + if (H5I_DATASET != it) + return FAIL; + + /*------------------------------------------------------------------------- + * attribute "DIMENSION_LABELS" + *------------------------------------------------------------------------- + */ + + /* try to find the attribute "DIMENSION_LABELS" on the >>data<< dataset */ + if ((has_labels = H5LT_find_attribute(did, DIMENSION_LABELS)) < 0) + return FAIL; + + /* return 0 and NULL for label if no label found */ + if (has_labels == 0) { + if (label) +/* label = NULL; */ + label[0] = 0; + return 0; + } - /* try to find the attribute "DIMENSION_LABELS" on the >>data<< dataset */ - if ((has_labels = H5LT_find_attribute(did,DIMENSION_LABELS)) < 0) - return FAIL; - - /* return 0 and NULL for label if no label found */ - if (has_labels == 0) - { - if (label) - label=NULL; - return 0; - } - - /* get dataset space */ - if ((sid = H5Dget_space(did)) < 0) - return FAIL; - - /* get rank */ - if((rank = H5Sget_simple_extent_ndims(sid)) < 0) - goto out; + /* get dataset space */ + if ((sid = H5Dget_space(did)) < 0) + return FAIL; - /* close dataset space */ - if(H5Sclose(sid) < 0) - goto out; + /* get rank */ + if ((rank = H5Sget_simple_extent_ndims(sid)) < 0) + goto out; -/*------------------------------------------------------------------------- - * open the attribute and read label - *------------------------------------------------------------------------- - */ + /* close dataset space */ + if (H5Sclose(sid) < 0) + goto out; - assert (has_labels == 1); - if((aid = H5Aopen(did, DIMENSION_LABELS, H5P_DEFAULT)) < 0) - goto out; + /*------------------------------------------------------------------------- + * open the attribute and read label + *------------------------------------------------------------------------- + */ - if((tid = H5Aget_type(aid)) < 0) - goto out; + assert (has_labels == 1); + if ((aid = H5Aopen(did, DIMENSION_LABELS, H5P_DEFAULT)) < 0) + goto out; - /* allocate and initialize */ - buf = (char **)malloc((size_t)rank * sizeof(char *)); + if ((tid = H5Aget_type(aid)) < 0) + goto out; - if(buf == NULL) - goto out; + /* allocate and initialize */ + buf = (char **) malloc((size_t) rank * sizeof(char *)); - /* read */ - if(H5Aread(aid, tid, buf) < 0) - goto out; + if (buf == NULL) + goto out; - /* get the real string length */ - nbytes = strlen(buf[idx]); + /* read */ + if (H5Aread(aid, tid, buf) < 0) + goto out; - /* compute the string length which will fit into the user's buffer */ - copy_len = MIN(size-1, nbytes); + /* get the real string length */ + nbytes = strlen(buf[idx]); - /* copy all/some of the name */ - if( label ) { - memcpy(label, buf[idx], copy_len); + /* compute the string length which will fit into the user's buffer */ + copy_len = MIN(size-1, nbytes); - /* terminate the string */ - label[copy_len]='\0'; + /* copy all/some of the name */ + if (label) { + memcpy(label, buf[idx], copy_len); - /* close */ - if (H5Tclose(tid) < 0) - goto out; - if (H5Aclose(aid) < 0) - goto out; - if (buf) - free(buf); - } + /* terminate the string */ + label[copy_len] = '\0'; + } - return (ssize_t) nbytes; + /* free all the ptr's from the H5Aread() */ + for (i = 0; i < (unsigned int) rank; i++) { + if (buf[i]) + free(buf[i]); + } - /* error zone, gracefully close */ -out: - H5E_BEGIN_TRY { - H5Sclose(sid); - H5Aclose(aid); - H5Tclose(tid); - } H5E_END_TRY; - return FAIL; + /* close */ + if (H5Tclose(tid) < 0) + goto out; + if (H5Aclose(aid) < 0) + goto out; + if (buf) + free(buf); + + return (ssize_t) nbytes; + + /* error zone, gracefully close */ + out: + if (buf) { + /* free all the ptr's from the H5Aread() */ + for (i = 0; i < (unsigned int) rank; i++) { + if (buf[i]) + free(buf[i]); + } + free(buf); + } + H5E_BEGIN_TRY + { + H5Sclose(sid); + H5Aclose(aid); + H5Tclose(tid); + }H5E_END_TRY; + return FAIL; } /*------------------------------------------------------------------------- -- cgit v0.12