From c462a2ec1f5af1f57935be0caa1209f0ae9d63c4 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Thu, 4 Feb 2010 21:56:25 -0500 Subject: [svn-r18212] Description: Bring revisions from Coverity fixing branch to trunk: r18184: Fixed Coverity issue 373. Allocated memory freed in line 762 in case of error. r18185: Fixed Coverity issues 357 & 358. Added check for NULL pointer before use. r18186: Fix coverity item 65. Added code to h5unjam to correctly handle failures in read() and write, and also to correctly handle writes that write less than requested. r18187: Fix coverity items 115 and 116. Added code to H5Tenum.c to correctly close opened datatypes in case of failure. r18188: Fixed Coverity issue 46. Check that dataset->shared is not null when freeing memory after error. r18190: Fix coverity item 95. Added code to H5T_create_vlen to correctly close allocated datatype in case of failure. r18191: Fixed Coverity error 59. Checked sfirst for -1 value before use in line 10533. r18192: Fix Coverity items 121 and 28 Added Asserts: 121: assert that all dimensions of count have values greater than zero. 28: assert curr_span pointer is not null before dereference. Note: still need too add checks in hyperslab APIs that fail when count values are zero, and appropriate tests. r18194: Fixed Coverity issues 61 & 62. Checked variable snpoints for value < 0 in line 218. Tested on: Mac OS X/32 10.6.2 (amazon) w/debug & production (already daily tested on coverity branch) --- fortran/src/H5Ef.c | 18 ++++++++------- hl/fortran/src/H5TBfc.c | 3 +++ src/H5Dcontig.c | 4 ++-- src/H5Dint.c | 22 +++++++++--------- src/H5Shyper.c | 21 +++++++++++------- src/H5Tconv.c | 2 ++ src/H5Tenum.c | 15 ++++++++----- src/H5Tvlen.c | 9 ++++++-- tools/h5jam/h5unjam.c | 59 +++++++++++++++++++++++++++++++++++++------------ 9 files changed, 103 insertions(+), 50 deletions(-) diff --git a/fortran/src/H5Ef.c b/fortran/src/H5Ef.c index e68c86c..c6d860c 100644 --- a/fortran/src/H5Ef.c +++ b/fortran/src/H5Ef.c @@ -128,10 +128,11 @@ nh5eget_major_c(int_f* error_no, _fcd name, size_t_f* namelen) size_t c_namelen = (size_t)*namelen; int_f ret_value = 0; - if(c_namelen) { - if(NULL == (c_name = (char *)HDmalloc(c_namelen + 1))) - HGOTO_DONE(FAIL) - } /* end if */ + if(c_namelen > 0) + c_name = (char *)HDmalloc(c_namelen + 1); + + if(!c_name) + HGOTO_DONE(FAIL) /* * Call H5Eget_major function. @@ -166,10 +167,11 @@ nh5eget_minor_c(int_f* error_no, _fcd name, size_t_f* namelen) size_t c_namelen = (size_t)*namelen; int_f ret_value = 0; - if(c_namelen) { - if(NULL == (c_name = (char *)HDmalloc(c_namelen + 1))) - HGOTO_DONE(FAIL) - } /* end if */ + if(c_namelen > 0) + c_name = (char *)HDmalloc(c_namelen + 1); + + if(!c_name) + HGOTO_DONE(FAIL) /* * Call H5Eget_minor function. diff --git a/hl/fortran/src/H5TBfc.c b/hl/fortran/src/H5TBfc.c index 7139be3..e942138 100755 --- a/hl/fortran/src/H5TBfc.c +++ b/hl/fortran/src/H5TBfc.c @@ -762,6 +762,9 @@ done: if(c_name) HDfree(c_name); + if(c_name1) + HDfree(c_name1); + return ret_value; } diff --git a/src/H5Dcontig.c b/src/H5Dcontig.c index e2fe420..7c1b87c 100644 --- a/src/H5Dcontig.c +++ b/src/H5Dcontig.c @@ -217,8 +217,8 @@ H5D_contig_fill(H5D_t *dset, hid_t dxpl_id) store.contig.dset_size = dset->shared->layout.storage.u.contig.size; /* Get the number of elements in the dataset's dataspace */ - snpoints = H5S_GET_EXTENT_NPOINTS(dset->shared->space); - HDassert(snpoints >= 0); + if((snpoints = H5S_GET_EXTENT_NPOINTS(dset->shared->space)) < 0) + HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "dataset has negative number of elements") H5_ASSIGN_OVERFLOW(npoints, snpoints, hssize_t, size_t); /* Initialize the fill value buffer */ diff --git a/src/H5Dint.c b/src/H5Dint.c index 3d5a5e7..bfe16f7 100644 --- a/src/H5Dint.c +++ b/src/H5Dint.c @@ -1309,17 +1309,19 @@ done: if(ret_value < 0) { if(H5F_addr_defined(dataset->oloc.addr) && H5O_close(&(dataset->oloc)) < 0) HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release object header") - if(dataset->shared->space && H5S_close(dataset->shared->space) < 0) - HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release dataspace") - if(dataset->shared->type) { - if(dataset->shared->type_id > 0) { - if(H5I_dec_ref(dataset->shared->type_id, FALSE) < 0) - HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release datatype") + if(dataset->shared) { + if(dataset->shared->space && H5S_close(dataset->shared->space) < 0) + HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release dataspace") + if(dataset->shared->type) { + if(dataset->shared->type_id > 0) { + if(H5I_dec_ref(dataset->shared->type_id, FALSE) < 0) + HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release datatype") + } /* end if */ + else { + if(H5T_close(dataset->shared->type) < 0) + HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release datatype") + } /* end else */ } /* end if */ - else { - if(H5T_close(dataset->shared->type) < 0) - HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release datatype") - } /* end else */ } /* end if */ } /* end if */ diff --git a/src/H5Shyper.c b/src/H5Shyper.c index 857df97..ea37e40 100644 --- a/src/H5Shyper.c +++ b/src/H5Shyper.c @@ -5450,6 +5450,8 @@ H5S_hyper_make_spans (unsigned rank, const hsize_t *start, const hsize_t *stride head = NULL; last_span = NULL; + HDassert(count[i] > 0); + /* Generate all the span segments for this dimension */ for(u = 0, stride_iter = 0; u < count[i]; u++, stride_iter += stride[i]) { /* Allocate a span node */ @@ -7492,23 +7494,26 @@ H5S_hyper_get_seq_list_gen(const H5S_t *space,H5S_sel_iter_t *iter, partial_done: /* Yes, goto's are evil, so sue me... :-) */ /* Perform the I/O on the elements, based on the position of the iterator */ - while(io_bytes_left>0 && curr_seq 0 && curr_seq < maxseq) { + /* Sanity check */ + HDassert(curr_span); + /* Adjust location offset of destination to compensate for initial increment below */ - loc_off-=curr_span->pstride; + loc_off -= curr_span->pstride; /* Loop over all the spans in the fastest changing dimension */ - while(curr_span!=NULL) { + while(curr_span != NULL) { /* Move location offset of destination */ - loc_off+=curr_span->pstride; + loc_off += curr_span->pstride; /* Compute the number of elements to attempt in this span */ - H5_ASSIGN_OVERFLOW(span_size,curr_span->nelem,hsize_t,size_t); + H5_ASSIGN_OVERFLOW(span_size, curr_span->nelem, hsize_t, size_t); /* Check number of elements against upper bounds allowed */ - if(span_size>=io_bytes_left) { + if(span_size >= io_bytes_left) { /* Trim the number of bytes to output */ - span_size=io_bytes_left; - io_bytes_left=0; + span_size = io_bytes_left; + io_bytes_left = 0; /* COMMON */ /* Store the I/O information for the span */ diff --git a/src/H5Tconv.c b/src/H5Tconv.c index bfa5d56..cda9011 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -10501,6 +10501,8 @@ H5T_conv_i_f (hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, sfirst = (ssize_t)(src.prec - 1); is_max_neg = 0; } + if(sfirst < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTCONVERT, FAIL, "zero bit not found") /* Sign bit has been negated if bit vector isn't 0x80...00. Set all bits in front of * sign bit to 0 in the temporary buffer because they're all negated from the previous diff --git a/src/H5Tenum.c b/src/H5Tenum.c index f955a7e..6da6931 100644 --- a/src/H5Tenum.c +++ b/src/H5Tenum.c @@ -466,10 +466,11 @@ H5T_enum_nameof(const H5T_t *dt, const void *value, char *name/*out*/, size_t si /* Set return value */ ret_value=name; - if (H5T_close(copied_dt)<0) - HGOTO_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close data type"); - done: + if(copied_dt) + if(H5T_close(copied_dt) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close data type"); + FUNC_LEAVE_NOAPI(ret_value) } @@ -591,9 +592,11 @@ H5T_enum_valueof(const H5T_t *dt, const char *name, void *value/*out*/) HDmemcpy(value, copied_dt->shared->u.enumer.value+md*copied_dt->shared->size, copied_dt->shared->size); - if (H5T_close(copied_dt)<0) - HGOTO_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, FAIL, "unable to close data type"); - done: + if(copied_dt) + if(H5T_close(copied_dt) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, FAIL, "unable to close data type") + FUNC_LEAVE_NOAPI(ret_value) } + diff --git a/src/H5Tvlen.c b/src/H5Tvlen.c index f09c42c..8a6ee05 100644 --- a/src/H5Tvlen.c +++ b/src/H5Tvlen.c @@ -161,7 +161,7 @@ H5T_vlen_create(const H5T_t *base) /* Build new type */ if(NULL == (dt = H5T_alloc())) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, NULL, "memory allocation failed") dt->shared->type = H5T_VLEN; /* @@ -169,7 +169,8 @@ H5T_vlen_create(const H5T_t *base) * data, not point to the same VL sequences) */ dt->shared->force_conv = TRUE; - dt->shared->parent = H5T_copy(base, H5T_COPY_ALL); + if(NULL == (dt->shared->parent = H5T_copy(base, H5T_COPY_ALL))) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTCOPY, NULL, "can't copy base datatype") /* Inherit encoding version from base type */ dt->shared->version = base->shared->version; @@ -185,6 +186,10 @@ H5T_vlen_create(const H5T_t *base) ret_value = dt; done: + if(!ret_value) + if(dt && H5T_close(dt) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTRELEASE, NULL, "unable to release datatype info") + FUNC_LEAVE_NOAPI(ret_value) } /* end H5T_vlen_create() */ diff --git a/tools/h5jam/h5unjam.c b/tools/h5jam/h5unjam.c index 3ab2202..5e4d864 100644 --- a/tools/h5jam/h5unjam.c +++ b/tools/h5jam/h5unjam.c @@ -28,10 +28,11 @@ #define TRUE 1 #define FALSE 0 +#define COPY_BUF_SIZE 1024 hsize_t write_pad( int , hsize_t ); hsize_t compute_pad( hsize_t ); -hsize_t copy_to_file( int , int , ssize_t, ssize_t ); +herr_t copy_to_file( int , int , ssize_t, ssize_t ); const char *progname = "h5unjam"; int d_status = EXIT_SUCCESS; @@ -268,13 +269,19 @@ main(int argc, const char *argv[]) /* copy from 0 to 'usize - 1' into ufid */ if (!do_delete) { - copy_to_file( ifid, ufid, 0, (ssize_t) usize); + if(copy_to_file(ifid, ufid, 0, (ssize_t) usize) < 0) { + error_msg(progname, "unable to copy user block to output file \"%s\"\n", ub_file); + exit(EXIT_FAILURE); + } } /* copy from usize to end of file into h5fid, * starting at end of user block if present */ - copy_to_file( ifid, h5fid, (ssize_t) usize, (ssize_t)(fsize - (ssize_t)usize) ); + if(copy_to_file(ifid, h5fid, (ssize_t) usize, (ssize_t)(fsize - (ssize_t)usize)) < 0) { + error_msg(progname, "unable to copy hdf5 data to output file \"%s\"\n", output_file); + exit(EXIT_FAILURE); + } HDclose(ufid); @@ -288,36 +295,60 @@ main(int argc, const char *argv[]) * Copy 'how_much' bytes from the input file to the output file, * starting at byte 'where' in the input file. * - * Returns the size of the output file. + * Returns 0 on success, -1 on failure. */ -hsize_t +herr_t copy_to_file( int infid, int ofid, ssize_t where, ssize_t how_much ) { - char buf[1024]; + static char buf[COPY_BUF_SIZE]; off_t to; off_t from; ssize_t nchars = -1; + ssize_t wnchars = -1; + herr_t ret_value = 0; /* nothing to copy */ if(how_much <= 0) - return(where); + goto done; from = where; to = 0; - while( how_much > 0) { + while(how_much > 0) { + /* Seek to correct position in input file */ HDlseek(infid,from,SEEK_SET); - if (how_much > 512) - nchars = HDread(infid,buf,(unsigned)512); + + /* Read data to buffer */ + if (how_much > COPY_BUF_SIZE) + nchars = HDread(infid,buf,(unsigned)COPY_BUF_SIZE); else nchars = HDread(infid,buf,(unsigned)how_much); + if(nchars < 0) { + ret_value = -1; + goto done; + } /* end if */ + + /* Seek to correct position in output file */ HDlseek(ofid,to,SEEK_SET); - HDwrite(ofid,buf,(unsigned)nchars); + + /* Update positions/size */ how_much -= nchars; from += nchars; to += nchars; - } - return (where + how_much); -} + /* Write nchars bytes to output file */ + wnchars = nchars; + while(nchars > 0) { + wnchars = HDwrite(ofid,buf,(unsigned)nchars); + if(wnchars < 0) { + ret_value = -1; + goto done; + } /* end if */ + nchars -= wnchars; + } /* end while */ + } /* end while */ + +done: + return ret_value; +} /* end copy_to_file */ -- cgit v0.12