From a976ea9f7774ad2f8b23ff5dc4a3b46469a383fb Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Sat, 6 Mar 2010 16:19:57 -0500 Subject: [svn-r18388] Description: Bring changes from Coverity session from branch into trunk: r18378: Fixed coverity issues 207 and 322. Pointer hdr was checked for NULL after being asserted and dereferenced. Check was removed. r18379: Fix coverity issues # 88 and # 435. r18380: Fixed Coverity issue # 85. Added check of returned pointer for NULL before use. r18381: Resolve coverity issues # 214 and # 215 r18382: Issue 131: Add null checks to allocations and check for free in error handling r18383: Issue 421: Reorganized code to make intention clearer. Also, set local variable fl to NULL after transfer to tail. Heap->freelist will take care of all allocations r18384: Coverity #249 and #250 - STRING_ATT_CHECK wasn't allocated before being used and freed in function test_write_vl_string_attribute and test_read_vl_string_attribute. Tested on: Mac OS X/32 10.6.2 (amazon) w/debug & prod (h5committested in daily tests) --- src/H5B2hdr.c | 2 +- src/H5Doh.c | 6 +- src/H5HFhdr.c | 2 +- src/H5HLcache.c | 6 +- src/H5trace.c | 149 ++--- test/dsets.c | 16 +- test/hyperslab.c | 1715 +++++++++++++++++++++++++++--------------------------- test/trefstr.c | 1 + test/tvlstr.c | 82 ++- 9 files changed, 1030 insertions(+), 949 deletions(-) diff --git a/src/H5B2hdr.c b/src/H5B2hdr.c index a4f7d8e..668bce4 100644 --- a/src/H5B2hdr.c +++ b/src/H5B2hdr.c @@ -599,7 +599,7 @@ H5B2_hdr_delete(H5B2_hdr_t *hdr, hid_t dxpl_id) done: /* Unprotect the header with appropriate flags */ - if(hdr && H5AC_unprotect(hdr->f, dxpl_id, H5AC_BT2_HDR, hdr->addr, hdr, cache_flags) < 0) + if(H5AC_unprotect(hdr->f, dxpl_id, H5AC_BT2_HDR, hdr->addr, hdr, cache_flags) < 0) HDONE_ERROR(H5E_BTREE, H5E_CANTUNPROTECT, FAIL, "unable to release B-tree header") FUNC_LEAVE_NOAPI(SUCCEED) diff --git a/src/H5Doh.c b/src/H5Doh.c index 98e9bf3..8ed6644 100644 --- a/src/H5Doh.c +++ b/src/H5Doh.c @@ -423,11 +423,11 @@ H5O_dset_bh_info(H5F_t *f, hid_t dxpl_id, H5O_t *oh, H5_ih_info_t *bh_info) done: /* Free messages, if they've been read in */ if(layout_read && H5O_msg_reset(H5O_LAYOUT_ID, &layout) < 0) - HGOTO_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset data storage layout message") + HDONE_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset data storage layout message") if(pline_read && H5O_msg_reset(H5O_PLINE_ID, &pline) < 0) - HGOTO_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset I/O pipeline message") + HDONE_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset I/O pipeline message") if(efl_read && H5O_msg_reset(H5O_EFL_ID, &efl) < 0) - HGOTO_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset external file list message") + HDONE_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset external file list message") FUNC_LEAVE_NOAPI(ret_value) } /* end H5O_dset_bh_info() */ diff --git a/src/H5HFhdr.c b/src/H5HFhdr.c index 8652f90..e55f472 100644 --- a/src/H5HFhdr.c +++ b/src/H5HFhdr.c @@ -1454,7 +1454,7 @@ H5HF_hdr_delete(H5HF_hdr_t *hdr, hid_t dxpl_id) done: /* Unprotect the header with appropriate flags */ - if(hdr && H5AC_unprotect(hdr->f, dxpl_id, H5AC_FHEAP_HDR, hdr->heap_addr, hdr, cache_flags) < 0) + if(H5AC_unprotect(hdr->f, dxpl_id, H5AC_FHEAP_HDR, hdr->heap_addr, hdr, cache_flags) < 0) HDONE_ERROR(H5E_HEAP, H5E_CANTUNPROTECT, FAIL, "unable to release fractal heap header") FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5HLcache.c b/src/H5HLcache.c index 6064fe5..47d5d6f 100644 --- a/src/H5HLcache.c +++ b/src/H5HLcache.c @@ -173,12 +173,12 @@ H5HL_fl_deserialize(H5HL_t *heap, hsize_t free_block) if(fl->offset + fl->size > heap->dblk_size) HGOTO_ERROR(H5E_HEAP, H5E_BADRANGE, FAIL, "bad heap free list") - /* Insert node into list */ + /* Append node onto list */ if(tail) tail->next = fl; - tail = fl; - if(!heap->freelist) + else heap->freelist = fl; + tail = fl; fl = NULL; } /* end while */ diff --git a/src/H5trace.c b/src/H5trace.c index b554b80..8c83505 100644 --- a/src/H5trace.c +++ b/src/H5trace.c @@ -916,172 +916,189 @@ H5_trace (const double *returning, const char *func, const char *type, ...) break; case 'i': - if (ptr) { - if (vp) { - fprintf (out, "0x%lx", (unsigned long)vp); - } else { + if(ptr) { + if(vp) + fprintf(out, "0x%lx", (unsigned long)vp); + else fprintf(out, "NULL"); - } - } else { + } /* end if */ + else { hid_t obj = va_arg (ap, hid_t); - if (H5P_DEFAULT == obj) { - fprintf (out, "H5P_DEFAULT"); - } else if (obj<0) { - fprintf (out, "FAIL"); - } else { - switch (H5I_TYPE(obj)) { /* Use internal H5I macro instead of function call */ + + if(H5P_DEFAULT == obj) + fprintf(out, "H5P_DEFAULT"); + else if(obj < 0) + fprintf(out, "FAIL"); + else { + switch(H5I_TYPE(obj)) { /* Use internal H5I macro instead of function call */ case H5I_UNINIT: - fprintf (out, "%ld (uninit - error)", (long)obj); + fprintf(out, "%ld (uninit - error)", (long)obj); break; + case H5I_BADID: - fprintf (out, "%ld (error)", (long)obj); + fprintf(out, "%ld (error)", (long)obj); break; + case H5I_FILE: fprintf(out, "%ld (file)", (long)obj); break; + case H5I_GROUP: fprintf(out, "%ld (group)", (long)obj); break; + case H5I_DATATYPE: - if (obj==H5T_NATIVE_SCHAR_g) { + if(obj == H5T_NATIVE_SCHAR_g) fprintf(out, "H5T_NATIVE_SCHAR"); - } else if (obj==H5T_NATIVE_UCHAR_g) { + else if(obj == H5T_NATIVE_UCHAR_g) fprintf(out, "H5T_NATIVE_UCHAR"); - } else if (obj==H5T_NATIVE_SHORT_g) { + else if(obj == H5T_NATIVE_SHORT_g) fprintf(out, "H5T_NATIVE_SHORT"); - } else if (obj==H5T_NATIVE_USHORT_g) { + else if(obj == H5T_NATIVE_USHORT_g) fprintf(out, "H5T_NATIVE_USHORT"); - } else if (obj==H5T_NATIVE_INT_g) { + else if(obj == H5T_NATIVE_INT_g) fprintf(out, "H5T_NATIVE_INT"); - } else if (obj==H5T_NATIVE_UINT_g) { + else if(obj == H5T_NATIVE_UINT_g) fprintf(out, "H5T_NATIVE_UINT"); - } else if (obj==H5T_NATIVE_LONG_g) { + else if(obj == H5T_NATIVE_LONG_g) fprintf(out, "H5T_NATIVE_LONG"); - } else if (obj==H5T_NATIVE_ULONG_g) { + else if(obj == H5T_NATIVE_ULONG_g) fprintf(out, "H5T_NATIVE_ULONG"); - } else if (obj==H5T_NATIVE_LLONG_g) { + else if(obj == H5T_NATIVE_LLONG_g) fprintf(out, "H5T_NATIVE_LLONG"); - } else if (obj==H5T_NATIVE_ULLONG_g) { + else if(obj == H5T_NATIVE_ULLONG_g) fprintf(out, "H5T_NATIVE_ULLONG"); - } else if (obj==H5T_NATIVE_FLOAT_g) { + else if(obj == H5T_NATIVE_FLOAT_g) fprintf(out, "H5T_NATIVE_FLOAT"); - } else if (obj==H5T_NATIVE_DOUBLE_g) { + else if(obj == H5T_NATIVE_DOUBLE_g) fprintf(out, "H5T_NATIVE_DOUBLE"); #if H5_SIZEOF_LONG_DOUBLE !=0 - } else if (obj==H5T_NATIVE_LDOUBLE_g) { + else if(obj == H5T_NATIVE_LDOUBLE_g) fprintf(out, "H5T_NATIVE_LDOUBLE"); #endif - } else if (obj==H5T_IEEE_F32BE_g) { + else if(obj == H5T_IEEE_F32BE_g) fprintf(out, "H5T_IEEE_F32BE"); - } else if (obj==H5T_IEEE_F32LE_g) { + else if(obj == H5T_IEEE_F32LE_g) fprintf(out, "H5T_IEEE_F32LE"); - } else if (obj==H5T_IEEE_F64BE_g) { + else if(obj == H5T_IEEE_F64BE_g) fprintf(out, "H5T_IEEE_F64BE"); - } else if (obj==H5T_IEEE_F64LE_g) { + else if(obj == H5T_IEEE_F64LE_g) fprintf(out, "H5T_IEEE_F64LE"); - } else if (obj==H5T_STD_I8BE_g) { + else if(obj == H5T_STD_I8BE_g) fprintf(out, "H5T_STD_I8BE"); - } else if (obj==H5T_STD_I8LE_g) { + else if(obj == H5T_STD_I8LE_g) fprintf(out, "H5T_STD_I8LE"); - } else if (obj==H5T_STD_I16BE_g) { + else if(obj == H5T_STD_I16BE_g) fprintf(out, "H5T_STD_I16BE"); - } else if (obj==H5T_STD_I16LE_g) { + else if(obj == H5T_STD_I16LE_g) fprintf(out, "H5T_STD_I16LE"); - } else if (obj==H5T_STD_I32BE_g) { + else if(obj == H5T_STD_I32BE_g) fprintf(out, "H5T_STD_I32BE"); - } else if (obj==H5T_STD_I32LE_g) { + else if(obj == H5T_STD_I32LE_g) fprintf(out, "H5T_STD_I32LE"); - } else if (obj==H5T_STD_I64BE_g) { + else if(obj == H5T_STD_I64BE_g) fprintf(out, "H5T_STD_I64BE"); - } else if (obj==H5T_STD_I64LE_g) { + else if(obj == H5T_STD_I64LE_g) fprintf(out, "H5T_STD_I64LE"); - } else if (obj==H5T_STD_U8BE_g) { + else if(obj == H5T_STD_U8BE_g) fprintf(out, "H5T_STD_U8BE"); - } else if (obj==H5T_STD_U8LE_g) { + else if(obj == H5T_STD_U8LE_g) fprintf(out, "H5T_STD_U8LE"); - } else if (obj==H5T_STD_U16BE_g) { + else if(obj == H5T_STD_U16BE_g) fprintf(out, "H5T_STD_U16BE"); - } else if (obj==H5T_STD_U16LE_g) { + else if(obj == H5T_STD_U16LE_g) fprintf(out, "H5T_STD_U16LE"); - } else if (obj==H5T_STD_U32BE_g) { + else if(obj == H5T_STD_U32BE_g) fprintf(out, "H5T_STD_U32BE"); - } else if (obj==H5T_STD_U32LE_g) { + else if(obj == H5T_STD_U32LE_g) fprintf(out, "H5T_STD_U32LE"); - } else if (obj==H5T_STD_U64BE_g) { + else if(obj == H5T_STD_U64BE_g) fprintf(out, "H5T_STD_U64BE"); - } else if (obj==H5T_STD_U64LE_g) { + else if(obj == H5T_STD_U64LE_g) fprintf(out, "H5T_STD_U64LE"); - } else if (obj==H5T_STD_B8BE_g) { + else if(obj == H5T_STD_B8BE_g) fprintf(out, "H5T_STD_B8BE"); - } else if (obj==H5T_STD_B8LE_g) { + else if(obj == H5T_STD_B8LE_g) fprintf(out, "H5T_STD_B8LE"); - } else if (obj==H5T_STD_B16BE_g) { + else if(obj == H5T_STD_B16BE_g) fprintf(out, "H5T_STD_B16BE"); - } else if (obj==H5T_STD_B16LE_g) { + else if(obj == H5T_STD_B16LE_g) fprintf(out, "H5T_STD_B16LE"); - } else if (obj==H5T_STD_B32BE_g) { + else if(obj == H5T_STD_B32BE_g) fprintf(out, "H5T_STD_B32BE"); - } else if (obj==H5T_STD_B32LE_g) { + else if(obj == H5T_STD_B32LE_g) fprintf(out, "H5T_STD_B32LE"); - } else if (obj==H5T_STD_B64BE_g) { + else if(obj == H5T_STD_B64BE_g) fprintf(out, "H5T_STD_B64BE"); - } else if (obj==H5T_STD_B64LE_g) { + else if(obj == H5T_STD_B64LE_g) fprintf(out, "H5T_STD_B64LE"); - } else if (obj==H5T_C_S1_g) { + else if(obj == H5T_C_S1_g) fprintf(out, "H5T_C_S1"); - } else if (obj==H5T_FORTRAN_S1_g) { + else if(obj == H5T_FORTRAN_S1_g) fprintf(out, "H5T_FORTRAN_S1"); - } else { + else fprintf(out, "%ld (dtype)", (long)obj); - } break; + case H5I_DATASPACE: fprintf(out, "%ld (dspace)", (long)obj); /* Save the rank of simple data spaces for arrays */ /* This may generate recursive call to the library... -QAK */ { - H5S_t *space = (H5S_t *)H5I_object(obj); - if (H5S_SIMPLE==H5S_GET_EXTENT_TYPE(space)) { - asize[argno] = H5S_GET_EXTENT_NDIMS(space); - } + H5S_t *space; + + if(NULL != (space = (H5S_t *)H5I_object(obj))) + if(H5S_SIMPLE == H5S_GET_EXTENT_TYPE(space)) + asize[argno] = H5S_GET_EXTENT_NDIMS(space); } break; + case H5I_DATASET: fprintf(out, "%ld (dset)", (long)obj); break; + case H5I_ATTR: fprintf(out, "%ld (attr)", (long)obj); break; + case H5I_REFERENCE: fprintf(out, "%ld (reference)", (long)obj); break; + case H5I_VFL: fprintf(out, "%ld (file driver)", (long)obj); break; + case H5I_GENPROP_CLS: fprintf(out, "%ld (genprop class)", (long)obj); break; + case H5I_GENPROP_LST: fprintf(out, "%ld (genprop list)", (long)obj); break; + case H5I_ERROR_CLASS: fprintf(out, "%ld (err class)", (long)obj); break; + case H5I_ERROR_MSG: fprintf(out, "%ld (err msg)", (long)obj); break; + case H5I_ERROR_STACK: fprintf(out, "%ld (err stack)", (long)obj); break; + case H5I_NTYPES: fprintf (out, "%ld (ntypes - error)", (long)obj); break; + default: fprintf(out, "%ld (unknown class)", (long)obj); break; - } - } - } + } /* end switch */ + } /* end else */ + } /* end else */ break; case 'I': diff --git a/test/dsets.c b/test/dsets.c index 4865ba9..46355c1 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -1274,8 +1274,6 @@ const H5Z_class2_t H5Z_CORRUPT[1] = {{ * Programmer: Raymond Lu * Jan 14, 2003 * - * Modifications: - * *------------------------------------------------------------------------- */ static size_t @@ -1283,7 +1281,7 @@ filter_corrupt(unsigned int flags, size_t cd_nelmts, const unsigned int *cd_values, size_t nbytes, size_t *buf_size, void **buf) { - void *data; + void *data = NULL; unsigned char *dst = (unsigned char*)(*buf); unsigned int offset; unsigned int length; @@ -1291,20 +1289,21 @@ filter_corrupt(unsigned int flags, size_t cd_nelmts, size_t ret_value = 0; if(cd_nelmts != 3 || !cd_values) - return 0; + TEST_ERROR offset = cd_values[0]; length = cd_values[1]; value = cd_values[2]; if(offset > nbytes || (offset + length) > nbytes || length < sizeof(unsigned int)) - return 0; + TEST_ERROR - data = HDmalloc((size_t)length); + if(NULL == (data = HDmalloc((size_t)length))) + TEST_ERROR HDmemset(data, (int)value, (size_t)length); if(flags & H5Z_FLAG_REVERSE) { /* Varify data is actually corrupted during read */ dst += offset; if(HDmemcmp(data, dst, (size_t)length) != 0) - ret_value = 0; + TEST_ERROR else { *buf_size = nbytes; ret_value = nbytes; @@ -1317,11 +1316,12 @@ filter_corrupt(unsigned int flags, size_t cd_nelmts, ret_value = *buf_size; } /* end else */ +error: if(data) HDfree(data); return ret_value; -} +} /* end filter_corrupt() */ /*------------------------------------------------------------------------- diff --git a/test/hyperslab.c b/test/hyperslab.c index b756449..e216b95 100644 --- a/test/hyperslab.c +++ b/test/hyperslab.c @@ -48,29 +48,27 @@ * Programmer: Robb Matzke * Friday, October 10, 1997 * - * Modifications: - * *------------------------------------------------------------------------- */ static unsigned init_full(uint8_t *array, size_t nx, size_t ny, size_t nz) { - size_t i, j, k; uint8_t acc = 128; unsigned total = 0; + size_t i, j, k; - for (i=0; i1) { + for(i = 0; i < nx; i++) { + if(nz > 1) printf("i=%lu:\n", (unsigned long)i); - } else { + else printf("%03lu:", (unsigned long)i); - } - for (j=0; j1) + for(j = 0; j < ny; j++) { + if(nz > 1) printf("%03lu:", (unsigned long)j); - for (k=0; k1) + if(nz > 1) printf("\n"); - } + } /* end for */ printf("\n"); - } -} + } /* end for */ +} /* end print_array() */ + /*------------------------------------------------------------------------- * Function: print_ref @@ -123,21 +118,21 @@ print_array(uint8_t *array, size_t nx, size_t ny, size_t nz) * Programmer: Robb Matzke * Friday, October 10, 1997 * - * Modifications: - * *------------------------------------------------------------------------- */ static void print_ref(size_t nx, size_t ny, size_t nz) { - uint8_t *array; + uint8_t *array; - array = HDcalloc(nx*ny*nz,sizeof(uint8_t)); + if(NULL != (array = HDmalloc(nx * ny * nz))) { + printf("Reference array:\n"); + init_full(array, nx, ny, nz); + print_array(array, nx, ny, nz); + HDfree(array); + } /* end if */ +} /* end print_ref() */ - printf("Reference array:\n"); - init_full(array, nx, ny, nz); - print_array(array, nx, ny, nz); -} /*------------------------------------------------------------------------- * Function: test_fill @@ -151,8 +146,6 @@ print_ref(size_t nx, size_t ny, size_t nz) * Programmer: Robb Matzke * Saturday, October 11, 1997 * - * Modifications: - * *------------------------------------------------------------------------- */ static herr_t @@ -160,142 +153,134 @@ test_fill(size_t nx, size_t ny, size_t nz, size_t di, size_t dj, size_t dk, size_t ddx, size_t ddy, size_t ddz) { - uint8_t *dst = NULL; /*destination array */ - hsize_t hs_size[3]; /*hyperslab size */ - hsize_t dst_size[3]; /*destination total size */ - hsize_t dst_offset[3]; /*offset of hyperslab in dest */ - unsigned ref_value; /*reference value */ - unsigned acc; /*accumulator */ - size_t i, j, k, dx, dy, dz; /*counters */ - size_t u, v, w; - unsigned ndims; /*hyperslab dimensionality */ - char dim[64], s[256]; /*temp string */ - unsigned fill_value; /*fill value */ + uint8_t *dst = NULL; /*destination array */ + hsize_t hs_size[3]; /*hyperslab size */ + hsize_t dst_size[3]; /*destination total size */ + hsize_t dst_offset[3]; /*offset of hyperslab in dest */ + unsigned ref_value; /*reference value */ + unsigned acc; /*accumulator */ + size_t i, j, k, dx, dy, dz; /*counters */ + size_t u, v, w; + unsigned ndims; /*hyperslab dimensionality */ + char dim[64], s[256]; /*temp string */ + unsigned fill_value; /*fill value */ /* * Dimensionality. */ - if (0 == nz) { - if (0 == ny) { - ndims = 1; - ny = nz = 1; - sprintf(dim, "%lu", (unsigned long) nx); - } else { - ndims = 2; - nz = 1; - sprintf(dim, "%lux%lu", (unsigned long) nx, (unsigned long) ny); - } - } else { - ndims = 3; - sprintf(dim, "%lux%lux%lu", - (unsigned long) nx, (unsigned long) ny, (unsigned long) nz); - } + if(0 == nz) { + if(0 == ny) { + ndims = 1; + ny = nz = 1; + sprintf(dim, "%lu", (unsigned long) nx); + } /* end if */ + else { + ndims = 2; + nz = 1; + sprintf(dim, "%lux%lu", (unsigned long) nx, (unsigned long) ny); + } /* end else */ + } /* end if */ + else { + ndims = 3; + sprintf(dim, "%lux%lux%lu", (unsigned long) nx, (unsigned long) ny, + (unsigned long) nz); + } /* end else */ sprintf(s, "Testing hyperslab fill %-11s variable hyperslab", dim); printf("%-70s", s); fflush(stdout); /* Allocate array */ - dst = HDcalloc((size_t)1, nx * ny * nz); + if(NULL == (dst = HDcalloc((size_t)1, nx * ny * nz))) + TEST_ERROR + init_full(dst, nx, ny, nz); - for (i = 0; i < nx; i += di) { - for (j = 0; j < ny; j += dj) { - for (k = 0; k < nz; k += dk) { - for (dx = 1; dx <= nx - i; dx += ddx) { - for (dy = 1; dy <= ny - j; dy += ddy) { - for (dz = 1; dz <= nz - k; dz += ddz) { - - /* Describe the hyperslab */ - dst_size[0] = nx; - dst_size[1] = ny; - dst_size[2] = nz; - dst_offset[0] = i; - dst_offset[1] = j; - dst_offset[2] = k; - hs_size[0] = dx; - hs_size[1] = dy; - hs_size[2] = dz; - - for (fill_value=0; - fill_value<256; - fill_value+=64) { - /* - * Initialize the full array, then subtract the - * original * fill values and add the new ones. - */ - ref_value = init_full(dst, nx, ny, nz); - for (u=(size_t)dst_offset[0]; - u