From 1d9307bd91d51e15e408a584b4c761cacf916b96 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Fri, 20 May 2011 17:04:03 -0500 Subject: [svn-r20885] Description: Bring Coverity changes back to trunk: r20340: Address coverity issue 248. Set freed pointer to NULL if realloc fails, then check for NULL before attempting to close array of handles stored in memory addressed by pointer. r20341: Added initialization udata.path = NULL; udata.visited = NULL; to prevent unintialized use after done: label. Do C-style initialization AFTER all variable definitions. r20342: Fix valgrind error in H5DUMP-XML-tall.h5 r20343: Addressed Coverity issue 591 - check the return value of H5S_SELECT_ITER_RELEASE. r20344: Added initialization udata.path = NULL; to prevent uninitialized used after done: label r20347: Purpose: Fix memory leak in overhead.c Description: Fixed leak in test() in overhead.c where array "had" was never freed. r20348: Addressed Coverity issue 705 - added prototype for resize_configs_are_equal to cache_common.h. r20392: Purpose: Fix coverity issues 1677 and 1678 Description: Added return value checking for 4 funciton calls in H5tools_get_symlink_info(). r20394: Purpose: Fix coverity issues 1674 and 1675 Description: Modified H5Epush2() and H5E_printf_stack() to keep track of whether va_start() has been called, and to call va_end() at the end if so. r20395: Should be H5MM_calloc(count * sizeof(void*)). Fixed --gh r20396: Should be H5MM_calloc(count * sizeof(void*)). Fixed. --gh Tested on: Mac OSX/32 10.6.7 (amazon) w/debug & production (h5committested on branch) --- perform/overhead.c | 7 +++++-- src/H5A.c | 2 +- src/H5Adeprec.c | 2 +- src/H5Dchunk.c | 14 +++++++------- src/H5E.c | 6 ++++-- src/H5Eint.c | 6 ++++-- src/H5G.c | 3 +++ src/H5Gname.c | 9 +++++++-- src/H5Gstab.c | 3 +++ src/H5MM.c | 14 ++++++-------- src/H5Oattribute.c | 2 +- src/H5Ztrans.c | 4 ++-- tools/h5dump/h5dump.c | 22 +++++++++++----------- tools/lib/h5tools_utils.c | 6 ++++-- 14 files changed, 59 insertions(+), 41 deletions(-) diff --git a/perform/overhead.c b/perform/overhead.c index 0c8e0ed..0b40721 100644 --- a/perform/overhead.c +++ b/perform/overhead.c @@ -243,7 +243,8 @@ test(fill_t fill_style, const double splits[], hs_start[0] = j%2 ? j/2 : (hssize_t)cur_size[0]-j/2; break; case FILL_RANDOM: - for (j=HDrand()%(int)cur_size[0]; had[j]; j=(j+1)%(int)cur_size[0]) /*void*/; + for (j=HDrand()%(int)cur_size[0]; had[j]; j=(j+1)%(int)cur_size[0]) + /*void*/; hs_start[0] = j; had[j] = 1; break; @@ -273,8 +274,10 @@ test(fill_t fill_style, const double splits[], } } - if(had) + if(had) { free(had); + had = NULL; + } /* end if */ H5Dclose(dset); H5Sclose(mspace); diff --git a/src/H5A.c b/src/H5A.c index 892e3bd..0abaa0a 100644 --- a/src/H5A.c +++ b/src/H5A.c @@ -547,7 +547,7 @@ H5Aopen(hid_t loc_id, const char *attr_name, hid_t UNUSED aapl_id) /* Read in attribute from object header */ if(NULL == (attr = H5O_attr_open_by_name(loc.oloc, attr_name, H5AC_ind_dxpl_id))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, FAIL, "unable to load attribute info from object header") + HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, FAIL, "unable to load attribute info from object header for attribute: '%s'", attr_name) /* Finish initializing attribute */ if(H5A_open_common(&loc, attr) < 0) diff --git a/src/H5Adeprec.c b/src/H5Adeprec.c index 2b244fd..b289ae0 100644 --- a/src/H5Adeprec.c +++ b/src/H5Adeprec.c @@ -211,7 +211,7 @@ H5Aopen_name(hid_t loc_id, const char *name) /* Open the attribute on the object header */ if(NULL == (attr = H5A_open_by_name(&loc, ".", name, H5P_LINK_ACCESS_DEFAULT, H5AC_ind_dxpl_id))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "can't open attribute") + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "can't open attribute: '%s'", name) /* Register the attribute and get an ID for it */ if((ret_value = H5I_register(H5I_ATTR, attr, TRUE)) < 0) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 643e1cb..0026183 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -3479,6 +3479,7 @@ H5D_chunk_prune_fill(H5D_chunk_it_ud1_t *udata) void *chunk; /* The file chunk */ H5D_chunk_ud_t chk_udata; /* User data for locking chunk */ uint32_t bytes_accessed; /* Bytes accessed in chunk */ + hbool_t chunk_iter_init = FALSE; /* Whether the chunk iterator has been initialized */ unsigned u; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ @@ -3544,16 +3545,11 @@ H5D_chunk_prune_fill(H5D_chunk_it_ud1_t *udata) /* Create a selection iterator for scattering the elements to memory buffer */ if(H5S_select_iter_init(&chunk_iter, udata->chunk_space, layout->u.chunk.dim[rank]) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to initialize chunk selection information") + chunk_iter_init = TRUE; /* Scatter the data into memory */ - if(H5D_scatter_mem(udata->fb_info.fill_buf, udata->chunk_space, &chunk_iter, (size_t)sel_nelmts, io_info->dxpl_cache, chunk/*out*/) < 0) { - H5S_SELECT_ITER_RELEASE(&chunk_iter); + if(H5D_scatter_mem(udata->fb_info.fill_buf, udata->chunk_space, &chunk_iter, (size_t)sel_nelmts, io_info->dxpl_cache, chunk/*out*/) < 0) HGOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "scatter failed") - } /* end if */ - - /* Release the selection iterator */ - if(H5S_SELECT_ITER_RELEASE(&chunk_iter) < 0) - HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "Can't release selection iterator") /* The number of bytes accessed in the chunk */ @@ -3566,6 +3562,10 @@ H5D_chunk_prune_fill(H5D_chunk_it_ud1_t *udata) HGOTO_ERROR(H5E_IO, H5E_WRITEERROR, FAIL, "unable to unlock raw data chunk") done: + /* Release the selection iterator */ + if(chunk_iter_init && H5S_SELECT_ITER_RELEASE(&chunk_iter) < 0) + HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "Can't release selection iterator") + FUNC_LEAVE_NOAPI(ret_value) } /* H5D_chunk_prune_fill */ diff --git a/src/H5E.c b/src/H5E.c index 7a81d32..af78f26 100644 --- a/src/H5E.c +++ b/src/H5E.c @@ -1333,6 +1333,7 @@ H5Epush2(hid_t err_stack, const char *file, const char *func, unsigned line, int desc_len; /* Actual length of description when formatted */ #endif /* H5_HAVE_VASPRINTF */ char *tmp = NULL; /* Buffer to place formatted description in */ + hbool_t va_started = FALSE; /* Whether the variable argument list is open */ herr_t ret_value=SUCCEED; /* Return value */ /* Don't clear the error stack! :-) */ @@ -1357,6 +1358,7 @@ H5Epush2(hid_t err_stack, const char *file, const char *func, unsigned line, /* Format the description */ va_start(ap, fmt); + va_started = TRUE; #ifdef H5_HAVE_VASPRINTF /* Use the vasprintf() routine, since it does what we're trying to do below */ @@ -1398,13 +1400,13 @@ H5Epush2(hid_t err_stack, const char *file, const char *func, unsigned line, } /* end while */ #endif /* H5_HAVE_VASPRINTF */ - va_end(ap); - /* Push the error on the stack */ if(H5E_push_stack(estack, file, func, line, cls_id, maj_id, min_id, tmp) < 0) HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, FAIL, "can't push error on stack") done: + if(va_started) + va_end(ap); if(tmp) H5MM_xfree(tmp); diff --git a/src/H5Eint.c b/src/H5Eint.c index f650581..10161ea 100644 --- a/src/H5Eint.c +++ b/src/H5Eint.c @@ -694,6 +694,7 @@ H5E_printf_stack(H5E_t *estack, const char *file, const char *func, unsigned lin int desc_len; /* Actual length of description when formatted */ #endif /* H5_HAVE_VASPRINTF */ char *tmp = NULL; /* Buffer to place formatted description in */ + hbool_t va_started = FALSE; /* Whether the variable argument list is open */ herr_t ret_value = SUCCEED; /* Return value */ /* @@ -718,6 +719,7 @@ H5E_printf_stack(H5E_t *estack, const char *file, const char *func, unsigned lin /* Start the variable-argument parsing */ va_start(ap, fmt); + va_started = TRUE; #ifdef H5_HAVE_VASPRINTF /* Use the vasprintf() routine, since it does what we're trying to do below */ @@ -759,13 +761,13 @@ H5E_printf_stack(H5E_t *estack, const char *file, const char *func, unsigned lin } /* end while */ #endif /* H5_HAVE_VASPRINTF */ - va_end(ap); - /* Push the error on the stack */ if(H5E_push_stack(estack, file, func, line, cls_id, maj_id, min_id, tmp) < 0) HGOTO_DONE(FAIL) done: + if(va_started) + va_end(ap); if(tmp) H5MM_xfree(tmp); diff --git a/src/H5G.c b/src/H5G.c index 1f561bd..1a0c35a 100644 --- a/src/H5G.c +++ b/src/H5G.c @@ -1737,6 +1737,9 @@ H5G_visit(hid_t loc_id, const char *group_name, H5_index_t idx_type, unsigned rc; /* Reference count of object */ herr_t ret_value; /* Return value */ + /* Portably clear udata struct (before FUNC_ENTER) */ + HDmemset(&udata, 0, sizeof(udata)); + FUNC_ENTER_NOAPI(H5G_visit, FAIL) /* Check args */ diff --git a/src/H5Gname.c b/src/H5Gname.c index c7feb62..2ba2d52 100644 --- a/src/H5Gname.c +++ b/src/H5Gname.c @@ -1105,7 +1105,8 @@ H5G_get_name_by_addr_cb(hid_t gid, const char *path, const H5L_info_t *linfo, /* Check for object in same file (handles mounted files) */ /* (re-verify address, in case we traversed a file mount) */ if(udata->loc->addr == obj_loc.oloc->addr && udata->loc->file == obj_loc.oloc->file) { - udata->path = H5MM_strdup(path); + if(NULL == (udata->path = H5MM_strdup(path))) + HGOTO_ERROR(H5E_SYM, H5E_CANTALLOC, H5_ITER_ERROR, "can't duplicate path string") /* We found a match so we return immediately */ HGOTO_DONE(H5_ITER_STOP) @@ -1145,6 +1146,9 @@ H5G_get_name_by_addr(hid_t file, hid_t lapl_id, hid_t dxpl_id, const H5O_loc_t * herr_t status; /* Status from iteration */ ssize_t ret_value; /* Return value */ + /* Portably clear udata struct (before FUNC_ENTER) */ + HDmemset(&udata, 0, sizeof(udata)); + FUNC_ENTER_NOAPI(H5G_get_name_by_addr, FAIL) /* Construct the link info for the file's root group */ @@ -1153,7 +1157,8 @@ H5G_get_name_by_addr(hid_t file, hid_t lapl_id, hid_t dxpl_id, const H5O_loc_t * /* Check for root group being the object looked for */ if(root_loc.oloc->addr == loc->addr && root_loc.oloc->file == loc->file) { - udata.path = H5MM_strdup(""); + if(NULL == (udata.path = H5MM_strdup(""))) + HGOTO_ERROR(H5E_SYM, H5E_CANTALLOC, FAIL, "can't duplicate path string") found_obj = TRUE; } /* end if */ else { diff --git a/src/H5Gstab.c b/src/H5Gstab.c index afa137c..001c463 100644 --- a/src/H5Gstab.c +++ b/src/H5Gstab.c @@ -714,6 +714,9 @@ H5G_stab_get_name_by_idx(H5O_loc_t *oloc, H5_iter_order_t order, hsize_t n, hbool_t udata_valid = FALSE; /* Whether iteration information is valid */ ssize_t ret_value; /* Return value */ + /* Portably clear udata struct (before FUNC_ENTER) */ + HDmemset(&udata, 0, sizeof(udata)); + FUNC_ENTER_NOAPI(H5G_stab_get_name_by_idx, FAIL) /* Sanity check */ diff --git a/src/H5MM.c b/src/H5MM.c index 1de4880..c27f9d1 100644 --- a/src/H5MM.c +++ b/src/H5MM.c @@ -110,7 +110,8 @@ H5MM_calloc(size_t size) * H5MM_realloc (NULL, 0) <==> NULL * * Return: Success: Ptr to new memory or NULL if the memory - * was freed. + * was freed or HDrealloc couldn't allocate + * memory. * * Failure: NULL * @@ -130,17 +131,14 @@ H5MM_realloc(void *mem, size_t size) if(NULL == mem) { if(0 == size) - mem = NULL; + ret_value = NULL; else - mem = H5MM_malloc(size); + ret_value = H5MM_malloc(size); } /* end if */ else if(0 == size) - mem = H5MM_xfree(mem); + ret_value = H5MM_xfree(mem); else - mem = HDrealloc(mem, size); - - /* Set return value */ - ret_value = mem; + ret_value = HDrealloc(mem, size); FUNC_LEAVE_NOAPI(ret_value) } /* end H5MM_realloc() */ diff --git a/src/H5Oattribute.c b/src/H5Oattribute.c index 4a19ed4..cafc664 100644 --- a/src/H5Oattribute.c +++ b/src/H5Oattribute.c @@ -527,7 +527,7 @@ H5O_attr_open_by_name(const H5O_loc_t *loc, const char *name, hid_t dxpl_id) /* Check that we found the attribute */ if(!udata.attr) - HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, NULL, "can't locate attribute") + HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, NULL, "can't locate attribute: '%s'", name) /* Get attribute opened from object header */ HDassert(udata.attr); diff --git a/src/H5Ztrans.c b/src/H5Ztrans.c index 8028123..e0f87ac 100644 --- a/src/H5Ztrans.c +++ b/src/H5Ztrans.c @@ -1383,7 +1383,7 @@ H5Z_xform_create(const char *expr) * we don't need to allocate any space since no array will have to be * stored */ if(count > 0) - if((data_xform_prop->dat_val_pointers->ptr_dat_val = (void**) H5MM_calloc(count * sizeof(void**))) == NULL) + if(NULL == (data_xform_prop->dat_val_pointers->ptr_dat_val = (void *)H5MM_calloc(count * sizeof(void *)))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "unable to allocate memory for pointers in transform array") /* Initialize the num_ptrs field, which will be used to keep track of the number of copies @@ -1518,7 +1518,7 @@ H5Z_xform_copy(H5Z_data_xform_t **data_xform_prop) count++; if(count > 0) - if((new_data_xform_prop->dat_val_pointers->ptr_dat_val = (void**) H5MM_calloc(count * sizeof(void**))) == NULL) + if(NULL == (new_data_xform_prop->dat_val_pointers->ptr_dat_val = (void *)H5MM_calloc(count * sizeof(void *)))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate memory for pointers in transform array") /* Zero out num_pointers prior to H5Z_xform_cop_tree call; that call will increment it to the right amount */ diff --git a/tools/h5dump/h5dump.c b/tools/h5dump/h5dump.c index cfeaaef..fcb7bd3 100644 --- a/tools/h5dump/h5dump.c +++ b/tools/h5dump/h5dump.c @@ -6193,7 +6193,7 @@ xml_dump_group(hid_t gid, const char *name) if(HDstrcmp(name, "/") == 0) { isRoot = 1; tmp = HDstrdup("/"); - } + } else { tmp = (char *)HDmalloc(HDstrlen(prefix) + HDstrlen(name) + 2); HDstrcpy(tmp, prefix); @@ -6213,7 +6213,7 @@ xml_dump_group(hid_t gid, const char *name) H5Oget_info(gid, &oinfo); if(oinfo.rc > 1) { - obj_t *found_obj; /* Found object */ + obj_t *found_obj; /* Found object */ /* Group with more than one link to it... */ found_obj = search_obj(group_table, oinfo.addr); @@ -6222,7 +6222,7 @@ xml_dump_group(hid_t gid, const char *name) indentation(indent); error_msg("internal error (file %s:line %d)\n", __FILE__, __LINE__); h5tools_setstatus(EXIT_FAILURE); - } + } else { char *t_name = xml_escape_the_name(name); char *grpxid = (char *)malloc(100); @@ -6237,7 +6237,7 @@ xml_dump_group(hid_t gid, const char *name) xml_name_to_XID("/", grpxid, 100, 1); HDfprintf(stdout, "<%sRootGroup OBJ-XID=\"%s\" H5Path=\"%s\">\n", xmlnsprefix, grpxid, "/"); - } + } else { t_objname = xml_escape_the_name(found_obj->objname); par_name = xml_escape_the_name(par); @@ -6263,7 +6263,7 @@ xml_dump_group(hid_t gid, const char *name) free(par_name); } free(ptrstr); - } + } else { /* first time this group has been seen -- describe it */ @@ -6271,7 +6271,7 @@ xml_dump_group(hid_t gid, const char *name) xml_name_to_XID("/", grpxid, 100, 1); HDfprintf(stdout, "<%sRootGroup OBJ-XID=\"%s\" H5Path=\"%s\">\n", xmlnsprefix, grpxid, "/"); - } + } else { char *t_tmp = xml_escape_the_name(tmp); @@ -6319,7 +6319,7 @@ xml_dump_group(hid_t gid, const char *name) /* iterate through all the links */ - if( (sort_by == H5_INDEX_CRT_ORDER) && (crt_order_flags & H5P_CRT_ORDER_TRACKED)) + if((sort_by == H5_INDEX_CRT_ORDER) && (crt_order_flags & H5P_CRT_ORDER_TRACKED)) H5Literate(gid, sort_by, sort_order, NULL, dump_all_cb, NULL); else H5Literate(gid, H5_INDEX_NAME, sort_order, NULL, dump_all_cb, NULL); @@ -6330,7 +6330,7 @@ xml_dump_group(hid_t gid, const char *name) free(grpxid); free(parentxid); } - } + } else { /* only link -- must be first time! */ @@ -6341,7 +6341,7 @@ xml_dump_group(hid_t gid, const char *name) if(isRoot) { xml_name_to_XID("/", grpxid, 100, 1); HDfprintf(stdout, "<%sRootGroup OBJ-XID=\"%s\" H5Path=\"%s\">\n", xmlnsprefix, grpxid, "/"); - } + } else { char *t_tmp = xml_escape_the_name(tmp); @@ -6403,11 +6403,11 @@ xml_dump_group(hid_t gid, const char *name) if(isRoot) HDfprintf(stdout, "\n", xmlnsprefix); else - HDfprintf(stdout, "\n" , xmlnsprefix); + HDfprintf(stdout, "\n", xmlnsprefix); if(par) free(par); if(tmp) - free(tmp); + free(tmp); } /*------------------------------------------------------------------------- diff --git a/tools/lib/h5tools_utils.c b/tools/lib/h5tools_utils.c index b1df66e..ee6d5d2 100644 --- a/tools/lib/h5tools_utils.c +++ b/tools/lib/h5tools_utils.c @@ -785,10 +785,12 @@ H5tools_get_symlink_info(hid_t file_id, const char * linkpath, h5tool_link_info_ * follow object in other file */ if(link_info->linfo.type == H5L_TYPE_EXTERNAL) { - fapl = H5Pcreate(H5P_FILE_ACCESS); + if((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0) + goto out; if(H5Pset_fapl_sec2(fapl) < 0) goto out; - lapl = H5Pcreate(H5P_LINK_ACCESS); + if((lapl = H5Pcreate(H5P_LINK_ACCESS)) < 0) + goto out; if(H5Pset_elink_fapl(lapl, fapl) < 0) goto out; } /* end if */ -- cgit v0.12