From 6488768a7aac69acb438ccbd3f9c560d0d3f819d Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 31 May 2018 12:55:52 -0600 Subject: [PATCH 01/12] Updated documentation for nc_inq_varid(). --- libdispatch/dvarinq.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libdispatch/dvarinq.c b/libdispatch/dvarinq.c index 4a8bd0441..dc239ad94 100644 --- a/libdispatch/dvarinq.c +++ b/libdispatch/dvarinq.c @@ -1,7 +1,7 @@ /*! \file Functions for inquiring about variables. -Copyright 2010 University Corporation for Atmospheric +Copyright 2018 University Corporation for Atmospheric Research/Unidata. See COPYRIGHT file for more info. */ @@ -38,6 +38,7 @@ ignored_if_null. \returns ::NC_NOERR No error. \returns ::NC_EBADID Bad ncid. +\returns ::NC_ENOTVAR Invalid variable ID. \section nc_inq_varid_example4 Example @@ -181,7 +182,7 @@ nc_inq_vartype(int ncid, int varid, nc_type *typep) NULL, NULL); } -/** +/** Learn how many dimensions are associated with a variable. \ingroup variables @@ -204,7 +205,7 @@ nc_inq_varndims(int ncid, int varid, int *ndimsp) return nc_inq_var(ncid, varid, NULL, NULL, ndimsp, NULL, NULL); } -/** +/** Learn the dimension IDs associated with a variable. \ingroup variables @@ -228,7 +229,7 @@ nc_inq_vardimid(int ncid, int varid, int *dimidsp) dimidsp, NULL); } -/** +/** Learn how many attributes are associated with a variable. \ingroup variables @@ -605,7 +606,7 @@ nc_inq_unlimdims(int ncid, int *nunlimdimsp, int *unlimdimidsp) #endif } -/** +/** Find the filter (if any) associated with a variable. This is a wrapper for nc_inq_var_all(). From 3e1bd3ce5f95452fbb79fdd659117ebd566a877b Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 31 May 2018 14:27:26 -0600 Subject: [PATCH 02/12] Removed some potential issues identified by static analysis. --- ncdump/nccopy.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ncdump/nccopy.c b/ncdump/nccopy.c index 619728673..f5f033a3a 100644 --- a/ncdump/nccopy.c +++ b/ncdump/nccopy.c @@ -272,9 +272,10 @@ parsefilterspec(const char* optarg0, struct FilterSpec* spec) } /* Check for special cases */ - if(strcmp(remainder,"none") == 0) { - spec->nofilter = 1; - goto done; + if( (remainder == NULL) || + (strncmp(remainder,"none",4) == 0)) { + spec->nofilter = 1; + goto done; } /* Collect the id+parameters */ @@ -741,7 +742,9 @@ copy_var_filter(int igrp, int varid, int ogrp, int o_varid) VarID vid = {igrp,varid}; VarID ovid = {ogrp,o_varid}; /* handle filter parameters, copying from input, overriding with command-line options */ - struct FilterSpec inspec, ospec, actualspec; + struct FilterSpec inspec = {NULL,0,0,0,NULL}, + ospec = {NULL,0,0,0,NULL}, + actualspec = {NULL,0,0,0,NULL}; int i; char* ofqn = NULL; int format, oformat; From db8d23b9862a3f57a0f9f95df139d6eba8810273 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 31 May 2018 15:09:11 -0600 Subject: [PATCH 03/12] Removed a potential double-free identified by static analysis. --- libsrc/nc3internal.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libsrc/nc3internal.c b/libsrc/nc3internal.c index 7cd665e34..fb99798d9 100644 --- a/libsrc/nc3internal.c +++ b/libsrc/nc3internal.c @@ -584,7 +584,7 @@ fill_added(NC3_INFO *gnu, NC3_INFO *old) { const NC_var *const gnu_varp = *(gnu_varpp + varid); - if (gnu_varp->no_fill) continue; + if (gnu_varp->no_fill) continue; if(IS_RECVAR(gnu_varp)) { @@ -1203,9 +1203,12 @@ NC3_open(const char * path, int ioflags, * !_CRAYMPP, only pe 0 is valid */ if(basepe != 0) { - if(nc3) free(nc3); - status = NC_EINVAL; - goto unwind_alloc; + if(nc3) { + free(nc3); + nc3 = NULL; + } + status = NC_EINVAL; + goto unwind_alloc; } #endif @@ -1743,7 +1746,7 @@ NC3_inq_format_extended(int ncid, int *formatp, int *modep) * Determine name and size of netCDF type. This netCDF-4 function * proved so popular that a netCDF-classic version is provided. You're * welcome. - * + * * \param ncid The ID of an open file. * \param typeid The ID of a netCDF type. * \param name Pointer that will get the name of the type. Maximum From 165d59e6afb97deccc289c5234a09d6c616af75c Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 31 May 2018 15:25:40 -0600 Subject: [PATCH 04/12] Fixed a potential memory leak identified by static analysis. --- libsrc4/nc4internal.c | 46 ++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/libsrc4/nc4internal.c b/libsrc4/nc4internal.c index bbaebad43..2a3d86623 100644 --- a/libsrc4/nc4internal.c +++ b/libsrc4/nc4internal.c @@ -13,7 +13,7 @@ * buffer of metadata information, i.e. the linked list of NC * structs. * - * @author Ed Hartnett + * @author Ed Hartnett, Dennis Heimbigner, Ward Fisher */ #include "config.h" #include "nc4internal.h" @@ -321,7 +321,7 @@ nc4_find_dim(NC_GRP_INFO_T *grp, int dimid, NC_DIM_INFO_T **dim, /* Find the dim info. */ (*dim) = nclistget(h5->alldims,dimid); - if((*dim) == NULL) + if((*dim) == NULL) return NC_EBADDIM; /* Redundant: Verify that this dim is in fact in the group or its parent */ @@ -488,7 +488,7 @@ nc4_find_grp_att(NC_GRP_INFO_T *grp, int varid, const char *name, int attnum, *att = a; return NC_NOERR; } - } + } /* If we get here, we couldn't find the attribute. */ return NC_ENOTATT; @@ -590,7 +590,7 @@ obj_track(NC_HDF5_FILE_INFO_T* file, NC_OBJ* obj) case NCGRP: list = file->allgroups; break; default: assert(NC_FALSE); - } + } /* Insert at the appropriate point in the list */ nclistset(list,obj->id,obj); } @@ -648,7 +648,7 @@ nc4_var_list_add(NC_GRP_INFO_T* grp, const char* name, int ndims, NC_VAR_INFO_T new_var->hdr.hashkey = NC_hashmapkey(new_var->hdr.name,strlen(new_var->hdr.name)); new_var->ndims = ndims; - + /* Allocate space for dimension information. */ if (ndims) { @@ -935,31 +935,33 @@ nc4_check_dup_name(NC_GRP_INFO_T *grp, char *name) * * @return ::NC_NOERR No error. * @return ::NC_ENOMEM Out of memory. - * @author Ed Hartnett + * @author Ed Hartnett, Ward Fisher */ int nc4_type_new(NC_GRP_INFO_T *grp, size_t size, const char *name, int assignedid, NC_TYPE_INFO_T **type) { - NC_TYPE_INFO_T *new_type; + NC_TYPE_INFO_T *new_type = NULL; - /* Allocate memory for the type */ - if (!(new_type = calloc(1, sizeof(NC_TYPE_INFO_T)))) - return NC_ENOMEM; - new_type->hdr.sort = NCTYP; + /* Allocate memory for the type */ + if (!(new_type = calloc(1, sizeof(NC_TYPE_INFO_T)))) + return NC_ENOMEM; + new_type->hdr.sort = NCTYP; - /* Remember info about this type. */ - new_type->hdr.id = assignedid; - new_type->size = size; - if (!(new_type->hdr.name = strdup(name))) - return NC_ENOMEM; + /* Remember info about this type. */ + new_type->hdr.id = assignedid; + new_type->size = size; + if (!(new_type->hdr.name = strdup(name))) { + free(new_type); + return NC_ENOMEM; + } - new_type->hdr.hashkey = NC_hashmapkey(name,strlen(name)); + new_type->hdr.hashkey = NC_hashmapkey(name,strlen(name)); - /* Return a pointer to the new type, if requested */ - if (type) - *type = new_type; + /* Return a pointer to the new type, if requested */ + if (type) + *type = new_type; - return NC_NOERR; + return NC_NOERR; } /** @@ -1328,7 +1330,7 @@ nc4_var_list_del(NC_GRP_INFO_T* grp, NC_VAR_INFO_T *var) } /** - * @internal Free a dim + * @internal Free a dim * * @param dim Pointer to dim info struct of type to delete. * From acc0b29140d086e027f2c0423565747c360b3212 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 31 May 2018 15:28:13 -0600 Subject: [PATCH 05/12] Potential dereference of a null pointer fixed. --- libsrc4/nc4internal.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libsrc4/nc4internal.c b/libsrc4/nc4internal.c index 2a3d86623..d8ed90993 100644 --- a/libsrc4/nc4internal.c +++ b/libsrc4/nc4internal.c @@ -1335,17 +1335,19 @@ nc4_var_list_del(NC_GRP_INFO_T* grp, NC_VAR_INFO_T *var) * @param dim Pointer to dim info struct of type to delete. * * @return ::NC_NOERR No error. - * @author Ed Hartnett + * @author Ed Hartnett, Ward Fisher */ int nc4_dim_free(NC_DIM_INFO_T *dim) { /* Free memory allocated for names. */ - if (dim->hdr.name) + if(dim) { + if (dim->hdr.name) free(dim->hdr.name); - free(dim); - return NC_NOERR; + free(dim); + } + return NC_NOERR; } /** From 3a0d55f13724522b230e9dda0c23f44ca609f0e7 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 31 May 2018 16:40:11 -0600 Subject: [PATCH 06/12] Removed a null assignment. --- libsrc4/nc4memcb.c | 218 ++++++++++++++++++++++----------------------- 1 file changed, 109 insertions(+), 109 deletions(-) diff --git a/libsrc4/nc4memcb.c b/libsrc4/nc4memcb.c index 3a4172c70..effe93032 100644 --- a/libsrc4/nc4memcb.c +++ b/libsrc4/nc4memcb.c @@ -18,7 +18,7 @@ /* * @internal Inmemory support - * + * * This code is derived from H5Lt.c#H5LTopen_file_image. * In order to make the netcdf inmemory code work, it is necessary * to modify some of the callback functions; specifically @@ -38,7 +38,7 @@ * The existing implementation of H5LTopen_file_image has two flaws * with respect to these properties. *
    - *
  1. The image_realloc callback fails if + *
  2. The image_realloc callback fails if * H5LT_FILE_IMAGE_DONT_COPY flag is set even if there is room * to allow the memory block to pretend to expand (because * of overallocation). @@ -135,7 +135,7 @@ static void tracefail(const char* fcn); /* user supplied image buffer. The same image is open with the core driver. */ #define H5LT_FILE_IMAGE_DONT_RELEASE 0x0004 /* The HDF5 lib won't */ /* deallocate user supplied image buffer. The user application is reponsible */ -/* for doing so. */ +/* for doing so. */ #define H5LT_FILE_IMAGE_ALL 0x0007 #endif /*H5LT_FILE_IMAGE_DONT_COPY*/ @@ -153,19 +153,19 @@ char *myinput; size_t indent = 0; #endif /*0*/ -/* File Image operations - - A file image is a representation of an HDF5 file in a memory - buffer. In order to perform operations on an image in a similar way +/* File Image operations + + A file image is a representation of an HDF5 file in a memory + buffer. In order to perform operations on an image in a similar way to a file, the application buffer is copied to a FAPL buffer, which - in turn is copied to a VFD buffer. Buffer copying can decrease - performance, especially when using large file images. A solution to - this issue is to simulate the copying of the application buffer, - when actually the same buffer is used for the FAPL and the VFD. - This is implemented by using callbacks that simulate the standard - functions for memory management (additional callbacks are used for - the management of associated data structures). From the application - standpoint, a file handle can be obtained from a file image by using + in turn is copied to a VFD buffer. Buffer copying can decrease + performance, especially when using large file images. A solution to + this issue is to simulate the copying of the application buffer, + when actually the same buffer is used for the FAPL and the VFD. + This is implemented by using callbacks that simulate the standard + functions for memory management (additional callbacks are used for + the management of associated data structures). From the application + standpoint, a file handle can be obtained from a file image by using the API routine H5LTopen_file_image(). This function takes a flag argument that indicates the HDF5 library how to handle the given image; several flag values can be combined by using the bitwise OR operator. @@ -185,10 +185,10 @@ size_t indent = 0; well. The application is responsible to release the image buffer. */ -/* Data structure to pass application data to callbacks. */ +/* Data structure to pass application data to callbacks. */ /* Modified to add NC_FILE_INFO_T ptr */ typedef struct { - void *app_image_ptr; /* Pointer to application buffer */ + void *app_image_ptr; /* Pointer to application buffer */ size_t app_image_size; /* Size of application buffer */ void *fapl_image_ptr; /* Pointer to FAPL buffer */ size_t fapl_image_size; /* Size of FAPL buffer */ @@ -213,17 +213,17 @@ static herr_t local_udata_free(void *udata); /*------------------------------------------------------------------------- -* Function: image_malloc +* Function: image_malloc * * Purpose: Simulates malloc() function to avoid copying file images. * The application buffer is set to the buffer on only one FAPL. * Then the FAPL buffer can be copied to other FAPL buffers or -* to only one VFD buffer. +* to only one VFD buffer. * * Return: Address of "allocated" buffer, if successful. Otherwise, it returns * NULL. * -* Programmer: Christian Chilan +* Programmer: Christian Chilan * * Date: October 3, 2011 * @@ -234,25 +234,25 @@ local_image_malloc(size_t size, H5FD_file_image_op_t file_image_op, void *_udata { H5LT_file_image_ud_t *udata = (H5LT_file_image_ud_t *)_udata; void * return_value = NULL; - + TRACE1("malloc",_udata, size); /* callback is only used if the application buffer is not actually copied */ - if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) + if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) goto out; switch ( file_image_op ) { /* the app buffer is "copied" to only one FAPL. Afterwards, FAPLs can be "copied" */ case H5FD_FILE_IMAGE_OP_PROPERTY_LIST_SET: - if (udata->app_image_ptr == NULL) + if (udata->app_image_ptr == NULL) goto out; - if (udata->app_image_size != size) + if (udata->app_image_size != size) goto out; - if (udata->fapl_image_ptr != NULL) + if (udata->fapl_image_ptr != NULL) goto out; - if (udata->fapl_image_size != 0) + if (udata->fapl_image_size != 0) goto out; - if (udata->fapl_ref_count != 0) + if (udata->fapl_ref_count != 0) goto out; udata->fapl_image_ptr = udata->app_image_ptr; @@ -262,11 +262,11 @@ local_image_malloc(size_t size, H5FD_file_image_op_t file_image_op, void *_udata break; case H5FD_FILE_IMAGE_OP_PROPERTY_LIST_COPY: - if (udata->fapl_image_ptr == NULL) + if (udata->fapl_image_ptr == NULL) goto out; - if (udata->fapl_image_size != size) + if (udata->fapl_image_size != size) goto out; - if (udata->fapl_ref_count == 0) + if (udata->fapl_ref_count == 0) goto out; return_value = udata->fapl_image_ptr; @@ -282,17 +282,17 @@ local_image_malloc(size_t size, H5FD_file_image_op_t file_image_op, void *_udata case H5FD_FILE_IMAGE_OP_FILE_OPEN: /* FAPL buffer is "copied" to only one VFD buffer */ - if (udata->vfd_image_ptr != NULL) + if (udata->vfd_image_ptr != NULL) goto out; - if (udata->vfd_image_size != 0) + if (udata->vfd_image_size != 0) goto out; - if (udata->vfd_ref_count != 0) + if (udata->vfd_ref_count != 0) goto out; - if (udata->fapl_image_ptr == NULL) + if (udata->fapl_image_ptr == NULL) goto out; - if (udata->fapl_image_size != size) + if (udata->fapl_image_size != size) goto out; - if (udata->fapl_ref_count == 0) + if (udata->fapl_ref_count == 0) goto out; udata->vfd_image_ptr = udata->fapl_image_ptr; @@ -323,15 +323,15 @@ out: /*------------------------------------------------------------------------- * Function: image_memcpy * -* Purpose: Simulates memcpy() function to avoid copying file images. +* Purpose: Simulates memcpy() function to avoid copying file images. * The image buffer can be set to only one FAPL buffer, and * "copied" to only one VFD buffer. The FAPL buffer can be -* "copied" to other FAPLs buffers. +* "copied" to other FAPLs buffers. * * Return: The address of the destination buffer, if successful. Otherwise, it * returns NULL. * -* Programmer: Christian Chilan +* Programmer: Christian Chilan * * Date: October 3, 2011 * @@ -346,36 +346,36 @@ local_image_memcpy(void *dest, const void *src, size_t size, H5FD_file_image_op_ TRACE3("memcpy",_udata,dest,src,size); /* callback is only used if the application buffer is not actually copied */ - if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) + if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) goto out; switch(file_image_op) { case H5FD_FILE_IMAGE_OP_PROPERTY_LIST_SET: - if (dest != udata->fapl_image_ptr) + if (dest != udata->fapl_image_ptr) goto out; - if (src != udata->app_image_ptr) + if (src != udata->app_image_ptr) goto out; - if (size != udata->fapl_image_size) + if (size != udata->fapl_image_size) goto out; - if (size != udata->app_image_size) + if (size != udata->app_image_size) goto out; - if (udata->fapl_ref_count == 0) + if (udata->fapl_ref_count == 0) goto out; break; case H5FD_FILE_IMAGE_OP_PROPERTY_LIST_COPY: - if (dest != udata->fapl_image_ptr) + if (dest != udata->fapl_image_ptr) goto out; - if (src != udata->fapl_image_ptr) + if (src != udata->fapl_image_ptr) goto out; - if (size != udata->fapl_image_size) + if (size != udata->fapl_image_size) goto out; - if (udata->fapl_ref_count < 2) + if (udata->fapl_ref_count < 2) goto out; break; case H5FD_FILE_IMAGE_OP_PROPERTY_LIST_GET: - if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) + if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) goto out; /* test: src == dest == original */ if(src != dest || src != udata->fapl_image_ptr) @@ -383,17 +383,17 @@ local_image_memcpy(void *dest, const void *src, size_t size, H5FD_file_image_op_ break; case H5FD_FILE_IMAGE_OP_FILE_OPEN: - if (dest != udata->vfd_image_ptr) + if (dest != udata->vfd_image_ptr) goto out; - if (src != udata->fapl_image_ptr) + if (src != udata->fapl_image_ptr) goto out; - if (size != udata->vfd_image_size) + if (size != udata->vfd_image_size) goto out; - if (size != udata->fapl_image_size) + if (size != udata->fapl_image_size) goto out; - if (udata->fapl_ref_count == 0) + if (udata->fapl_ref_count == 0) goto out; - if (udata->vfd_ref_count != 1) + if (udata->vfd_ref_count != 1) goto out; break; @@ -419,15 +419,15 @@ out: /*------------------------------------------------------------------------- -* Function: image_realloc +* Function: image_realloc * * Purpose: Reallocates the shared application image buffer and updates data -* structures that manage buffer "copying". -* -* Return: Address of reallocated buffer, if successful. Otherwise, it returns -* NULL. +* structures that manage buffer "copying". * -* Programmer: Christian Chilan +* Return: Address of reallocated buffer, if successful. Otherwise, it returns +* NULL. +* +* Programmer: Christian Chilan * * Date: October 3, 2011 * @@ -442,21 +442,21 @@ local_image_realloc(void *ptr, size_t size, H5FD_file_image_op_t file_image_op, TRACE2("realloc",_udata, ptr, size); /* callback is only used if the application buffer is not actually copied */ - if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) + if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) goto out; /* realloc() is not allowed if the image is open in read-only mode */ - if (!(udata->flags & H5LT_FILE_IMAGE_OPEN_RW)) - goto out; + if (!(udata->flags & H5LT_FILE_IMAGE_OPEN_RW)) + goto out; if (file_image_op == H5FD_FILE_IMAGE_OP_FILE_RESIZE) { - if (udata->vfd_image_ptr != ptr) - goto out; - - if (udata->vfd_ref_count != 1) + if (udata->vfd_image_ptr != ptr) goto out; - /* Modified: + if (udata->vfd_ref_count != 1) + goto out; + + /* Modified: 1. If the realloc new size is <= existing size, then pretend we did a realloc and return success. This avoids unneccessary heap operations. @@ -476,7 +476,7 @@ local_image_realloc(void *ptr, size_t size, H5FD_file_image_op_t file_image_op, goto out; /* realloc MAY violate these flags */ } else { if (NULL == (udata->vfd_image_ptr = HDrealloc(ptr, size))) - goto out; + goto out; udata->vfd_image_size = size; } return_value = udata->vfd_image_ptr; @@ -502,9 +502,9 @@ out: * reference counters. Shared application buffer is actually * deallocated if there are no outstanding references. * -* Return: SUCCEED or FAIL +* Return: SUCCEED or FAIL * -* Programmer: Christian Chilan +* Programmer: Christian Chilan * * Date: October 3, 2011 * @@ -518,19 +518,19 @@ local_image_free(void *ptr, H5FD_file_image_op_t file_image_op, void *_udata) TRACE1("free",_udata,ptr); /* callback is only used if the application buffer is not actually copied */ - if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) + if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) goto out; switch(file_image_op) { case H5FD_FILE_IMAGE_OP_PROPERTY_LIST_CLOSE: - if (udata->fapl_image_ptr != ptr) + if (udata->fapl_image_ptr != ptr) goto out; - if (udata->fapl_ref_count == 0) + if (udata->fapl_ref_count == 0) goto out; udata->fapl_ref_count--; - /* release the shared buffer only if indicated by the respective flag and there are no outstanding references */ + /* release the shared buffer only if indicated by the respective flag and there are no outstanding references */ if (udata->fapl_ref_count == 0 && udata->vfd_ref_count == 0 && !(udata->flags & H5LT_FILE_IMAGE_DONT_RELEASE)) { free(udata->fapl_image_ptr); @@ -541,14 +541,14 @@ local_image_free(void *ptr, H5FD_file_image_op_t file_image_op, void *_udata) break; case H5FD_FILE_IMAGE_OP_FILE_CLOSE: - if (udata->vfd_image_ptr != ptr) + if (udata->vfd_image_ptr != ptr) goto out; - if (udata->vfd_ref_count != 1) + if (udata->vfd_ref_count != 1) goto out; udata->vfd_ref_count--; - /* release the shared buffer only if indicated by the respective flag and there are no outstanding references */ + /* release the shared buffer only if indicated by the respective flag and there are no outstanding references */ if (udata->fapl_ref_count == 0 && udata->vfd_ref_count == 0 && !(udata->flags & H5LT_FILE_IMAGE_DONT_RELEASE)) { free(udata->vfd_image_ptr); @@ -579,7 +579,7 @@ out: /*------------------------------------------------------------------------- -* Function: udata_copy +* Function: udata_copy * * Purpose: Simulates the copying of the user data structure utilized in the * management of the "copying" of file images. @@ -587,7 +587,7 @@ out: * Return: Address of "newly allocated" structure, if successful. Otherwise, it * returns NULL. * -* Programmer: Christian Chilan +* Programmer: Christian Chilan * * Date: October 3, 2011 * @@ -599,10 +599,10 @@ local_udata_copy(void *_udata) H5LT_file_image_ud_t *udata = (H5LT_file_image_ud_t *)_udata; /* callback is only used if the application buffer is not actually copied */ - if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) + if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) goto out; - if (udata->ref_count == 0) + if (udata->ref_count == 0) goto out; udata->ref_count++; @@ -619,11 +619,11 @@ out: * * Purpose: Simulates deallocation of the user data structure utilized in the * management of the "copying" of file images. The data structure is -* actually deallocated when there are no outstanding references. +* actually deallocated when there are no outstanding references. * -* Return: SUCCEED or FAIL +* Return: SUCCEED or FAIL * -* Programmer: Christian Chilan +* Programmer: Christian Chilan * * Date: October 3, 2011 * @@ -635,10 +635,10 @@ local_udata_free(void *_udata) H5LT_file_image_ud_t *udata = (H5LT_file_image_ud_t *)_udata; /* callback is only used if the application buffer is not actually copied */ - if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) + if (!(udata->flags & H5LT_FILE_IMAGE_DONT_COPY)) goto out; - if (udata->ref_count == 0) + if (udata->ref_count == 0) goto out; udata->ref_count--; @@ -650,7 +650,7 @@ local_udata_free(void *_udata) return(SUCCEED); -out: +out: return(FAIL); } /* end udata_free */ @@ -666,11 +666,11 @@ out: /*------------------------------------------------------------------------- * Function: H5LTopen_file_image * -* Purpose: Open a user supplied file image using the core file driver. +* Purpose: Open a user supplied file image using the core file driver. * * Return: File identifier, Failure: -1 * -* Programmer: Christian Chilan +* Programmer: Christian Chilan * * Date: October 3, 2011 * @@ -686,20 +686,20 @@ NC4_image_init(NC_HDF5_FILE_INFO_T* h5) size_t min_incr = 65536; /* Minimum buffer increment */ double buf_prcnt = 0.1f; /* Percentage of buffer size to set as increment */ - size_t buf_size = h5->mem.memio.size; + size_t buf_size = 0; void* buf_ptr = h5->mem.memio.memory; unsigned flags = h5->mem.flags; - static long file_name_counter; - H5FD_file_image_callbacks_t callbacks = {&local_image_malloc, &local_image_memcpy, - &local_image_realloc, &local_image_free, - &local_udata_copy, &local_udata_free, + static long file_name_counter; + H5FD_file_image_callbacks_t callbacks = {&local_image_malloc, &local_image_memcpy, + &local_image_realloc, &local_image_free, + &local_udata_copy, &local_udata_free, (void *)NULL}; /* check arguments */ if (buf_ptr == NULL) { if(h5->mem.created) { if(h5->mem.memio.size == 0) h5->mem.memio.size = DEFAULT_CREATE_MEMSIZE; - h5->mem.memio.memory = malloc(h5->mem.memio.size); + h5->mem.memio.memory = malloc(h5->mem.memio.size); } else goto out; /* open requires an input buffer */ } @@ -707,13 +707,13 @@ NC4_image_init(NC_HDF5_FILE_INFO_T* h5) buf_size = h5->mem.memio.size; buf_ptr = h5->mem.memio.memory; /* validate */ - if (buf_ptr == NULL || buf_size == 0) + if (buf_ptr == NULL || buf_size == 0) goto out; if (flags & (unsigned)~(H5LT_FILE_IMAGE_ALL)) goto out; /* Create FAPL to transmit file image */ - if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0) + if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0) goto out; /* set allocation increment to a percentage of the supplied buffer size, or @@ -725,7 +725,7 @@ NC4_image_init(NC_HDF5_FILE_INFO_T* h5) alloc_incr = min_incr; /* Configure FAPL to use the core file driver */ - if (H5Pset_fapl_core(fapl, alloc_incr, FALSE) < 0) + if (H5Pset_fapl_core(fapl, alloc_incr, FALSE) < 0) goto out; /* Set callbacks for file image ops ONLY if the file image is NOT copied */ @@ -762,10 +762,10 @@ NC4_image_init(NC_HDF5_FILE_INFO_T* h5) } /* Assign file image in user buffer to FAPL */ - if (H5Pset_file_image(fapl, buf_ptr, buf_size) < 0) + if (H5Pset_file_image(fapl, buf_ptr, buf_size) < 0) goto out; - /* set file open flags */ + /* set file open flags */ if (flags & H5LT_FILE_IMAGE_OPEN_RW) file_open_flags = H5F_ACC_RDWR; else @@ -773,21 +773,21 @@ NC4_image_init(NC_HDF5_FILE_INFO_T* h5) /* define a unique file name */ snprintf(file_name, (sizeof(file_name) - 1), "file_image_%ld", file_name_counter++); - - /* Assign file image in FAPL to the core file driver */ + + /* Assign file image in FAPL to the core file driver */ if(h5->mem.created) { file_open_flags |= H5F_ACC_TRUNC; - if ((file_id = H5Fcreate(file_name, file_open_flags, H5P_DEFAULT, fapl)) < 0) + if ((file_id = H5Fcreate(file_name, file_open_flags, H5P_DEFAULT, fapl)) < 0) goto out; } else { - if ((file_id = H5Fopen(file_name, file_open_flags, fapl)) < 0) + if ((file_id = H5Fopen(file_name, file_open_flags, fapl)) < 0) goto out; } h5->mem.fapl = fapl; - /* Return file identifier */ - return file_id; + /* Return file identifier */ + return file_id; out: H5E_BEGIN_TRY { From 3cce9e99e2b3111f5d8378128fd6e58393d0c7f4 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 7 Jun 2018 15:21:42 -0600 Subject: [PATCH 07/12] Corrected several potential null dereference or memory leak issues detected by static analysis. --- libdispatch/dauth.c | 74 ++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/libdispatch/dauth.c b/libdispatch/dauth.c index 484c87df3..e03b91baf 100644 --- a/libdispatch/dauth.c +++ b/libdispatch/dauth.c @@ -91,8 +91,9 @@ NC_authsetup(NCauth* auth, NCURI* uri) char* uri_hostport = NULL; if(uri != NULL) - uri_hostport = NC_combinehostport(uri); - + uri_hostport = NC_combinehostport(uri); + else + return NC_EDAP; /* Generic EDAP error. */ setdefaults(auth); /* Note, we still must do this function even if @@ -136,32 +137,34 @@ NC_authsetup(NCauth* auth, NCURI* uri) NC_rclookup("HTTP.SSL.VERIFYPEER",uri_hostport)); setauthfield(auth,"HTTP.NETRC", NC_rclookup("HTTP.NETRC",uri_hostport)); + { /* Handle various cases for user + password */ - /* First, see if the user+pwd was in the original url */ - char* user = NULL; - char* pwd = NULL; - if(uri->user != NULL && uri->password != NULL) { + /* First, see if the user+pwd was in the original url */ + char* user = NULL; + char* pwd = NULL; + if(uri->user != NULL && uri->password != NULL) { user = uri->user; pwd = uri->password; - } else { + } else { user = NC_rclookup("HTTP.CREDENTIALS.USER",uri_hostport); pwd = NC_rclookup("HTTP.CREDENTIALS.PASSWORD",uri_hostport); - } - if(user != NULL && pwd != NULL) { - user = strdup(user); /* so we can consistently reclaim */ - pwd = strdup(pwd); - } else { + } + if(user != NULL && pwd != NULL) { + user = strdup(user); /* so we can consistently reclaim */ + pwd = strdup(pwd); + } else { /* Could not get user and pwd, so try USERPASSWORD */ const char* userpwd = NC_rclookup("HTTP.CREDENTIALS.USERPASSWORD",uri_hostport); if(userpwd != NULL) { - ret = NC_parsecredentials(userpwd,&user,&pwd); - if(ret) return ret; + ret = NC_parsecredentials(userpwd,&user,&pwd); + if(ret) {nullfree(uri_hostport); return ret;} } - } - setauthfield(auth,"HTTP.USERNAME",user); - setauthfield(auth,"HTTP.PASSWORD",pwd); - nullfree(user); - nullfree(pwd); + } + setauthfield(auth,"HTTP.USERNAME",user); + setauthfield(auth,"HTTP.PASSWORD",pwd); + nullfree(user); + nullfree(pwd); + nullfree(uri_hostport); } return (ret); } @@ -347,25 +350,27 @@ and do %xx unescaping int NC_parsecredentials(const char* userpwd, char** userp, char** pwdp) { - char* user = NULL; - char* pwd = NULL; + char* user = NULL; + char* pwd = NULL; - if(userpwd == NULL) + if(userpwd == NULL) return NC_EINVAL; - user = strdup(userpwd); - if(user == NULL) + user = strdup(userpwd); + if(user == NULL) return NC_ENOMEM; - pwd = strchr(user,':'); - if(pwd == NULL) - return NC_EINVAL; - *pwd = '\0'; - pwd++; - if(userp) - *userp = ncuridecode(user); - if(pwdp) - *pwdp = ncuridecode(pwd); + pwd = strchr(user,':'); + if(pwd == NULL) { free(user); - return NC_NOERR; + return NC_EINVAL; + } + *pwd = '\0'; + pwd++; + if(userp) + *userp = ncuridecode(user); + if(pwdp) + *pwdp = ncuridecode(pwd); + free(user); + return NC_NOERR; } static void @@ -380,4 +385,3 @@ setdefaults(NCauth* auth) } } } - From bb65fe5ea507c0f2962dc707ce928c723d770daa Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 7 Jun 2018 15:58:47 -0600 Subject: [PATCH 08/12] More potential memory leaks squashed. --- libdispatch/dutf8.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/libdispatch/dutf8.c b/libdispatch/dutf8.c index 08bea64ee..447a42b4d 100644 --- a/libdispatch/dutf8.c +++ b/libdispatch/dutf8.c @@ -126,47 +126,51 @@ int nc_utf8_to_utf16(const unsigned char* s8, unsigned short** utf16p, size_t* l len8 = strlen((char*)s8); utf16 = (unsigned short*)malloc(sizeof(unsigned short)*(len8+1)); if(utf16 == NULL) { - ncstat = NC_ENOMEM; - goto done; + ncstat = NC_ENOMEM; + goto done; } str = (const nc_utf8proc_uint8_t*)s8; /* Walk the string and convert each codepoint */ p16 = utf16; len16 = 0; while(*str) { - count = nc_utf8proc_iterate(str,nchars,&codepoint); - if(count < 0) { + count = nc_utf8proc_iterate(str,nchars,&codepoint); + if(count < 0) { switch (count) { case UTF8PROC_ERROR_NOMEM: case UTF8PROC_ERROR_OVERFLOW: - ncstat = NC_ENOMEM; - break; + ncstat = NC_ENOMEM; + break; case UTF8PROC_ERROR_INVALIDOPTS: - ncstat = NC_EINVAL; - break; + ncstat = NC_EINVAL; + break; case UTF8PROC_ERROR_INVALIDUTF8: case UTF8PROC_ERROR_NOTASSIGNED: default: - ncstat = NC_EBADNAME; - break; + ncstat = NC_EBADNAME; + break; } goto done; - } else { /* move to next char */ + } else { /* move to next char */ /* Complain if top 16 bits not zero */ if((codepoint & 0x0000FFFF) != 0) { - ncstat = NC_EBADNAME; - goto done; + ncstat = NC_EBADNAME; + goto done; } /* Truncate codepoint to 16 bits and store */ *p16++ = (unsigned short)(codepoint & 0x0000FFFF); str += count; len16++; - } + } } *p16++ = (unsigned short)0; - if(utf16p) *utf16p = utf16; + if(utf16p) + *utf16p = utf16; + else + free(utf16); + if(len16p) *len16p = len16; -done: + done: if(ncstat) free(utf16); return ncstat; } From 1af8b8f8fcfbfd848d82e536c39835f43ede34c6 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 7 Jun 2018 16:17:32 -0600 Subject: [PATCH 09/12] Corrected a stored value that's never read issue detected by static analysis. --- libdispatch/drc.c | 50 +++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/libdispatch/drc.c b/libdispatch/drc.c index b20e3ed2b..644c83df8 100644 --- a/libdispatch/drc.c +++ b/libdispatch/drc.c @@ -142,7 +142,7 @@ NC_set_rcfile(const char* rcfile) nullfree(ncrc_globalstate.rcinfo.rcfile); ncrc_globalstate.rcinfo.rcfile = strdup(rcfile); /* Clear ncrc_globalstate.rcinfo */ - NC_rcclear(&ncrc_globalstate.rcinfo); + NC_rcclear(&ncrc_globalstate.rcinfo); /* (re) load the rcfile and esp the triplestore*/ stat = NC_rcload(); done: @@ -196,10 +196,9 @@ static void rctrim(char* text) { char* p = text; - size_t len; + size_t len = 0; int i; - len = strlen(text); /* locate first non-trimchar */ for(;*p;p++) { if(strchr(TRIMCHARS,*p) == NULL) break; /* hit non-trim char */ @@ -264,8 +263,8 @@ rccompile(const char* path) char* nextline = NULL; if((ret=NC_readfile(path,tmp))) { - nclog(NCLOGERR, "Could not open configuration file: %s",path); - goto done; + nclog(NCLOGERR, "Could not open configuration file: %s",path); + goto done; } contents = ncbytesextract(tmp); if(contents == NULL) contents = strdup(""); @@ -292,27 +291,29 @@ rccompile(const char* path) if((llen=strlen(line)) == 0) continue; /* empty line */ triple = (NCTriple*)calloc(1,sizeof(NCTriple)); if(triple == NULL) {ret = NC_ENOMEM; goto done;} - if(line[0] == LTAG) { - char* url = ++line; - char* rtag = strchr(line,RTAG); - if(rtag == NULL) { - nclog(NCLOGERR, "Malformed [url] in %s entry: %s",path,line); - continue; - } - line = rtag + 1; - *rtag = '\0'; - /* compile the url and pull out the host */ - if(uri) ncurifree(uri); - if(ncuriparse(url,&uri) != NCU_OK) { - nclog(NCLOGERR, "Malformed [url] in %s entry: %s",path,line); + if(line[0] == LTAG) { + char* url = ++line; + char* rtag = strchr(line,RTAG); + if(rtag == NULL) { + nclog(NCLOGERR, "Malformed [url] in %s entry: %s",path,line); + free(triple); + continue; + } + line = rtag + 1; + *rtag = '\0'; + /* compile the url and pull out the host */ + if(uri) ncurifree(uri); + if(ncuriparse(url,&uri) != NCU_OK) { + nclog(NCLOGERR, "Malformed [url] in %s entry: %s",path,line); + free(triple); continue; - } - ncbytesclear(tmp); - ncbytescat(tmp,uri->host); - if(uri->port != NULL) { + } + ncbytesclear(tmp); + ncbytescat(tmp,uri->host); + if(uri->port != NULL) { ncbytesappend(tmp,':'); - ncbytescat(tmp,uri->port); - } + ncbytescat(tmp,uri->port); + } ncbytesnull(tmp); triple->host = ncbytesextract(tmp); if(strlen(triple->host)==0) @@ -441,4 +442,3 @@ storedump(char* msg, NClist* triples) fflush(stderr); } #endif - From 595f39ce4239999b03fc91d20640c08eb83c8fe3 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Fri, 8 Jun 2018 11:18:08 -0600 Subject: [PATCH 10/12] Corrected an unlikely but possible passage of a null pointer to memcpy. --- libhdf5/nc4hdf.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/libhdf5/nc4hdf.c b/libhdf5/nc4hdf.c index 93a7d899e..9cf63e9ea 100644 --- a/libhdf5/nc4hdf.c +++ b/libhdf5/nc4hdf.c @@ -584,7 +584,7 @@ check_for_vara(nc_type *mem_nc_type, NC_VAR_INFO_T *var, NC_HDF5_FILE_INFO_T *h5 #ifdef LOGGING /** - * @intarnal Print some debug info about dimensions to the log. + * @intarnal Print some debug info about dimensions to the log. */ static void log_dim_info(NC_VAR_INFO_T *var, hsize_t *fdims, hsize_t *fmaxdims, @@ -1290,8 +1290,10 @@ nc4_get_vara(NC *nc, int ncid, int varid, const size_t *startp, } else { *(char **)filldata = NULL; } - } else - memcpy(filldata, fillvalue, file_type_size); + } else { + if(fillvalue) + memcpy(filldata, fillvalue, file_type_size); + } filldata = (char *)filldata + file_type_size; } } @@ -1335,7 +1337,7 @@ exit: } /** - * @internal Write an attribute. + * @internal Write an attribute. * * @param grp Pointer to group info struct. * @param varid Variable ID or NC_GLOBAL. @@ -1462,7 +1464,7 @@ exit: } /** - * @internal Write all the dirty atts in an attlist. + * @internal Write all the dirty atts in an attlist. * * @param attlist Pointer to the list if attributes. * @param varid Variable ID. @@ -1506,7 +1508,7 @@ write_attlist(NCindex* attlist, int varid, NC_GRP_INFO_T *grp) * next time. This function also contains a new way of dealing with * HDF5 error handling, abandoning the BAIL macros for a more organic * and natural approach, made with whole grains, and locally-grown - * vegetables. + * vegetables. * * @param var Pointer to var info struct. * @@ -1535,7 +1537,7 @@ write_coord_dimids(NC_VAR_INFO_T *var) } /** - * @internal Write a special attribute for the netCDF-4 dimension ID. + * @internal Write a special attribute for the netCDF-4 dimension ID. * * @param datasetid HDF5 datasset ID. * @param dimid NetCDF dimension ID. @@ -1583,7 +1585,7 @@ write_netcdf4_dimid(hid_t datasetid, int dimid) } /** - * @internal This function creates the HDF5 dataset for a variable. + * @internal This function creates the HDF5 dataset for a variable. * * @param grp Pointer to group info struct. * @param var Pointer to variable info struct. @@ -1847,7 +1849,7 @@ exit: /** * @internal Adjust the chunk cache of a var for better - * performance. + * performance. * * @param grp Pointer to group info struct. * @param var Pointer to var info struct. @@ -1895,7 +1897,7 @@ nc4_adjust_var_cache(NC_GRP_INFO_T *grp, NC_VAR_INFO_T * var) /** * @internal Create a HDF5 defined type from a NC_TYPE_INFO_T struct, - * and commit it to the file. + * and commit it to the file. * * @param grp Pointer to group info struct. * @param type Pointer to type info struct. @@ -2028,7 +2030,7 @@ commit_type(NC_GRP_INFO_T *grp, NC_TYPE_INFO_T *type) /** * @internal Write an attribute, with value 1, to indicate that strict - * NC3 rules apply to this file. + * NC3 rules apply to this file. * * @param hdf_grpid HDF5 group ID. * @@ -2127,9 +2129,9 @@ exit: * @internal After all the datasets of the file have been read, it's * time to sort the wheat from the chaff. Which of the datasets are * netCDF dimensions, and which are coordinate variables, and which - * are non-coordinate variables. + * are non-coordinate variables. * - * @param grp Pointer to group info struct. + * @param grp Pointer to group info struct. * * @return ::NC_NOERR No error. * @author Ed Hartnett @@ -2288,7 +2290,7 @@ remove_coord_atts(hid_t hdf_datasetid) * @internal This function writes a variable. The principle difficulty * comes from the possibility that this is a coordinate variable, and * was already written to the file as a dimension-only dimscale. If - * this occurs, then it must be deleted and recreated. + * this occurs, then it must be deleted and recreated. * * @param var Pointer to variable info struct. * @param grp Pointer to group info struct. @@ -2627,7 +2629,7 @@ exit: * group or any subgroups, to find out if we have to handle that * situation. Also check if there are any multidimensional coordinate * variables defined, which require the same treatment to fix a - * potential bug when such variables occur in subgroups. + * potential bug when such variables occur in subgroups. * * @param grp Pointer to group info struct. * @param bad_coord_orderp Pointer that gets 1 if there is a bad @@ -2783,7 +2785,7 @@ nc4_rec_write_metadata(NC_GRP_INFO_T *grp, nc_bool_t bad_coord_order) } /** - * @internal Recursively write all groups and types. + * @internal Recursively write all groups and types. * * @param grp Pointer to group info struct. * @@ -3646,7 +3648,7 @@ nc4_convert_type(const void *src, void *dest, const nc_type src_type, * @internal In our first pass through the data, we may have * encountered variables before encountering their dimscales, so go * through the vars in this file and make sure we've got a dimid for - * each. + * each. * * @param grp Pointer to group info struct. * @@ -3836,7 +3838,7 @@ nc4_rec_match_dimscales(NC_GRP_INFO_T *grp) /** * @internal Get the length, in bytes, of one element of a type in - * memory. + * memory. * * @param h5 Pointer to HDF5 file info struct. * @param xtype NetCDF type ID. @@ -3901,7 +3903,7 @@ nc4_get_typelen_mem(NC_HDF5_FILE_INFO_T *h5, nc_type xtype, size_t *len) } /** - * @internal Get the class of a type + * @internal Get the class of a type * * @param h5 Pointer to the HDF5 file info struct. * @param xtype NetCDF type ID. @@ -4014,7 +4016,7 @@ reportobject(int uselog, hid_t id, unsigned int type) { fprintf(stderr,"Type = %s(%lld) name='%s'",typename,printid,name); } - + } /** From 335f25e947e5eda1846103fb7d96e4dc285afaf0 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Fri, 8 Jun 2018 11:38:11 -0600 Subject: [PATCH 11/12] Corrected a couple of potential memory related issues and an uninitialized variable issue. --- libdispatch/dfile.c | 20 +++++++++++++------- ncgen/genbin.c | 4 ++-- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/libdispatch/dfile.c b/libdispatch/dfile.c index 62f9fd196..576912dcc 100644 --- a/libdispatch/dfile.c +++ b/libdispatch/dfile.c @@ -2192,13 +2192,19 @@ NC_open(const char *path0, int cmode, int basepe, size_t *chunksizehintp, #ifdef USE_CDF5 cdf5built = 1; #endif - if(!hdf5built && model == NC_FORMATX_NC4) - return NC_ENOTBUILT; - if(!hdf4built && model == NC_FORMATX_NC4 && version == 4) - return NC_ENOTBUILT; - if(!cdf5built && model == NC_FORMATX_NC3 && version == 5) - return NC_ENOTBUILT; - } + if(!hdf5built && model == NC_FORMATX_NC4) { + free(path); + return NC_ENOTBUILT; + } + if(!hdf4built && model == NC_FORMATX_NC4 && version == 4) { + free(path); + return NC_ENOTBUILT; + } + if(!cdf5built && model == NC_FORMATX_NC3 && version == 5) { + free(path); + return NC_ENOTBUILT; + } + } /* Force flag consistentcy */ if(model == NC_FORMATX_NC4 || model == NC_FORMATX_NC_HDF4 || model == NC_FORMATX_DAP4) diff --git a/ncgen/genbin.c b/ncgen/genbin.c index 29df7793d..56b35bea4 100644 --- a/ncgen/genbin.c +++ b/ncgen/genbin.c @@ -198,7 +198,7 @@ genbin_defineglobalspecials(void) static void genbin_definespecialattributes(Symbol* var) { - int stat; + int stat = NC_NOERR; Specialdata* special = &var->var.special; if(special->flags & _STORAGE_FLAG) { int storage = special->_Storage; @@ -250,7 +250,7 @@ genbin_definespecialattributes(Symbol* var) else level = special->_FilterParams[0]; if(level > 9) - derror("Illegal deflate level"); + derror("Illegal deflate level"); else { stat = nc_def_var_deflate(var->container->ncid, var->ncid, From 4fba5dfb4512499c8731bf0f3fbed506da8c1212 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Fri, 8 Jun 2018 12:08:33 -0600 Subject: [PATCH 12/12] Squaed a potential memory leak. --- libdispatch/ncuri.c | 48 ++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/libdispatch/ncuri.c b/libdispatch/ncuri.c index 09584274d..b2d86db83 100644 --- a/libdispatch/ncuri.c +++ b/libdispatch/ncuri.c @@ -191,7 +191,7 @@ ncuriparse(const char* uri0, NCURI** durip) } } else { /*c == '#'*/ tmp.fragment = next; - } + } } /* Parse the prefix parameters */ @@ -217,7 +217,7 @@ ncuriparse(const char* uri0, NCURI** durip) nclistpush(querylist,NULL); tmp.querylist = nclistextract(querylist); } - } + } /* Now parse the core of the url */ p = tmp.uri; @@ -245,7 +245,7 @@ ncuriparse(const char* uri0, NCURI** durip) Note in all cases, the leading '/' is considered part of the path, which is then assumed to be an absolute path. But also note that the windows drive letter has to be taken into account. Our rule is that - if the path looks like D:..., + if the path looks like D:..., where D is a single alphabetic letter (a-z or A-Z), then it is a windows path and can be use in place of a /path. The rules implemented here (for file:) are then as follows @@ -319,7 +319,7 @@ ncuriparse(const char* uri0, NCURI** durip) {THROW(NCU_EUSRPWD);} /* we have empty user */ if(strlen(pp)==0) {THROW(NCU_EUSRPWD);} /* we have empty password */ - tmp.password = pp; + tmp.password = pp; tmp.host = newhost; } /* Breakup host into host + port */ @@ -351,7 +351,7 @@ ncuriparse(const char* uri0, NCURI** durip) /* before saving, we need to decode the user+pwd */ duri->user = NULL; duri->password = NULL; - if(tmp.user != NULL) + if(tmp.user != NULL) duri->user = ncuridecode(tmp.user); if(tmp.password != NULL) duri->password = ncuridecode(tmp.password); @@ -368,7 +368,10 @@ ncuriparse(const char* uri0, NCURI** durip) duri->fragment = nulldup(tmp.fragment); duri->fraglist = tmp.fraglist; tmp.fraglist = NULL; duri->querylist = tmp.querylist; tmp.querylist = NULL; - if(durip) *durip = duri; + if(durip) + *durip = duri; + else + free(duri); #ifdef NCXDEBUG { @@ -387,7 +390,8 @@ ncuriparse(const char* uri0, NCURI** durip) done: if(uri != NULL) - free(uri); + free(uri); + freestringlist(params); freestringlist(querylist); freestringvec(tmp.fraglist); @@ -412,10 +416,10 @@ static void freestringvec(char** list) { if(list != NULL) { - char** p; + char** p; for(p=list;*p;p++) {nullfree(*p);} nullfree(list); - } + } } void @@ -450,7 +454,7 @@ int ncurisetquery(NCURI* duri,const char* query) { int ret = NCU_OK; - freestringvec(duri->querylist); + freestringvec(duri->querylist); nullfree(duri->query); duri->query = NULL; duri->querylist = NULL; @@ -560,7 +564,7 @@ ncuribuild(NCURI* duri, const char* prefix, const char* suffix, int flags) char* encoded = ncuriencodeonly(duri->path,pathallow); ncbytescat(buf,encoded); nullfree(encoded); - } else + } else ncbytescat(buf,duri->path); } @@ -582,10 +586,10 @@ ncuribuild(NCURI* duri, const char* prefix, const char* suffix, int flags) char* encoded = ncuriencodeonly(p[1],queryallow); ncbytescat(buf,encoded); nullfree(encoded); - } else + } else ncbytescat(buf,p[1]); } - } + } } if((flags & NCURIFRAG) && duri->fraglist != NULL) { char** p; @@ -599,10 +603,10 @@ ncuribuild(NCURI* duri, const char* prefix, const char* suffix, int flags) char* encoded = ncuriencodeonly(p[1],queryallow); ncbytescat(buf,encoded); nullfree(encoded); - } else + } else ncbytescat(buf,p[1]); } - } + } } ncbytesnull(buf); newuri = ncbytesextract(buf); @@ -647,7 +651,7 @@ ncuriremoveparam(NCURI* uri, const char* key) if(uri->fraglist == NULL) return NCU_OK; for(q=uri->fraglist,p=uri->fraglist;*p;) { if(strcmp(key,*p)==0) { - p += 2; /* skip this entry */ + p += 2; /* skip this entry */ } else { *q++ = *p++; /* move key */ *q++ = *p++; /* move value */ @@ -903,7 +907,7 @@ collectprefixparams(char* text, char** nextp) last = ep; /* save this position */ ep++; /* move past rbracket */ sp = ep; - } + } /* nul terminate */ if(last != NULL) terminate(last); @@ -914,7 +918,7 @@ collectprefixparams(char* text, char** nextp) char* p; char* q; /* by construction, here we are at an LBRACKET: compress it out */ for(p=sp,q=sp+1;(*p++=*q++);) - ; + ; /* locate the next RRACKET */ ep = nclocate(sp,RBRACKETSTR); if(ep == NULL) break;/* we are done */ @@ -922,9 +926,9 @@ collectprefixparams(char* text, char** nextp) *ep = '&'; ep++; /* move past rbracket */ sp = ep; - } + } done: - return ret; + return ret; } static int @@ -945,10 +949,10 @@ parselist(char* ptext, NClist* list) if(ep != NULL) { terminate(ep); /* overwrite the trailing ampersand */ p = ep+1; /* next param */ - } + } /* split into key + value */ eq = strchr(sp,'='); - if(eq != NULL) { /* value is present */ + if(eq != NULL) { /* value is present */ terminate(eq); eq++; key = strdup(sp); value = strdup(eq);