From b113f6f8b6effb1578e753db9d660e8d7490152f Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Tue, 16 Apr 2013 23:02:54 +0000 Subject: [PATCH 1/2] Merged a handful of changes from netcdf-cmake branch. Addressed the following coverity issues: 711762 711763 711766 711788 711933 711934 711935 --- include/ncconfigure.h | 8 +++----- libdap2/dceparse.c | 4 ++-- libdispatch/ncbytes.c | 11 +++++++---- libsrc4/nc4internal.c | 5 +++++ ncgen/cvt.c | 31 +++++++++++++++++++++---------- ncgen/data.c | 3 +++ oc2/xxdr.c | 1 + 7 files changed, 42 insertions(+), 21 deletions(-) diff --git a/include/ncconfigure.h b/include/ncconfigure.h index 48f586865..bf7986cd0 100644 --- a/include/ncconfigure.h +++ b/include/ncconfigure.h @@ -23,13 +23,11 @@ extern char* strdup(const char*); #endif #if HAVE_BASETSD_H +#ifndef SSIZE_T #include #endif - -//#ifndef SIZEOF_SSIZE_T -//#undef ssize_t -//#define ssize_t int -//#endif +#define ssize_t SSIZE_T +#endif diff --git a/libdap2/dceparse.c b/libdap2/dceparse.c index d5cacd9bd..956f21911 100644 --- a/libdap2/dceparse.c +++ b/libdap2/dceparse.c @@ -124,7 +124,7 @@ Object range(DCEparsestate* state, Object sfirst, Object sstride, Object slast) { DCEslice* slice = (DCEslice*)dcecreate(CES_SLICE); - unsigned long first,stride,last; + unsigned long first=0,stride=0,last=0; /* Note: that incoming arguments are strings; we must convert to size_t; but we do know they are legal integers or NULL */ @@ -143,7 +143,7 @@ range(DCEparsestate* state, Object sfirst, Object sstride, Object slast) if(last < first) dceerror(state,"Illegal index for range last index"); slice->first = first; - slice->stride = stride; + slice->stride = (stride == 0 ? 1 : stride); slice->stop = last + 1; slice->length = slice->stop - slice->first; slice->count = slice->length / slice->stride; diff --git a/libdispatch/ncbytes.c b/libdispatch/ncbytes.c index 67ef56f0b..5dde17ab5 100644 --- a/libdispatch/ncbytes.c +++ b/libdispatch/ncbytes.c @@ -123,11 +123,14 @@ ncbytesappend(NCbytes* bb, char elem) int ncbytescat(NCbytes* bb, const char* s) { - ncbytesappendn(bb,(void*)s,strlen(s)+1); /* include trailing null*/ - /* back up over the trailing null*/ - if(bb->length == 0) return ncbytesfail(); - bb->length--; + if(s == NULL) { return 1; + } + ncbytesappendn(bb,(void*)s,strlen(s)+1); /* include trailing null*/ + /* back up over the trailing null*/ + if(bb->length == 0) return ncbytesfail(); + bb->length--; + return 1; } int diff --git a/libsrc4/nc4internal.c b/libsrc4/nc4internal.c index b8fdd836f..aab809f2c 100644 --- a/libsrc4/nc4internal.c +++ b/libsrc4/nc4internal.c @@ -327,6 +327,11 @@ nc4_find_g_var_nc(NC *nc, int ncid, int varid, assert(grp && var && h5 && h5->root_grp); *grp = nc4_rec_find_grp(h5->root_grp, (ncid & GRP_ID_MASK)); + /* It is possible for *grp to be NULL. If it is, + return an error. */ + if(*grp == NULL) + return NC_ENOTVAR; + /* Find the var info. */ for ((*var) = (*grp)->var; (*var); (*var) = (*var)->next) if ((*var)->varid == varid) diff --git a/ncgen/cvt.c b/ncgen/cvt.c index a1927712a..bab362f06 100644 --- a/ncgen/cvt.c +++ b/ncgen/cvt.c @@ -491,38 +491,49 @@ case CASE(NC_DOUBLE,NC_STRING): break; case CASE(NC_OPAQUE,NC_CHAR): + if(bytes) tmp.charv = *(char*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_BYTE): + if(bytes) tmp.uint8v = *(unsigned char*)bytes; break; case CASE(NC_OPAQUE,NC_UBYTE): + if(bytes) tmp.uint8v = *(unsigned char*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_USHORT): + if(bytes) tmp.uint16v = *(unsigned short*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_UINT): + if(bytes) tmp.uint32v = *(unsigned int*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_UINT64): + if(bytes) tmp.uint64v = *(unsigned long long*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_SHORT): + if(bytes) tmp.int16v = *(short*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_INT): + if(bytes) tmp.int32v = *(int*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_INT64): + if(bytes) tmp.int64v = *(long long*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_FLOAT): + if(bytes) tmp.floatv = *(float*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_DOUBLE): + if(bytes) tmp.doublev = *(double*)bytes; - break; + break; case CASE(NC_OPAQUE,NC_OPAQUE): tmp.opaquev.stringv = (char*)malloc(src->value.opaquev.len+1); memcpy(tmp.opaquev.stringv,src->value.opaquev.stringv,src->value.opaquev.len); diff --git a/ncgen/data.c b/ncgen/data.c index d4887aacf..0fe8f984c 100644 --- a/ncgen/data.c +++ b/ncgen/data.c @@ -609,6 +609,7 @@ bbprintf(Bytebuffer* buf, const char *fmt, ...) va_list argv; va_start(argv,fmt); vbbprintf(buf,fmt,argv); + va_end(argv); } void @@ -618,6 +619,7 @@ bbprintf0(Bytebuffer* buf, const char *fmt, ...) va_start(argv,fmt); bbClear(buf); vbbprintf(buf,fmt,argv); + va_end(argv); } void @@ -626,6 +628,7 @@ codeprintf(const char *fmt, ...) va_list argv; va_start(argv,fmt); vbbprintf(codebuffer,fmt,argv); + va_end(argv); } Constant* diff --git a/oc2/xxdr.c b/oc2/xxdr.c index eefddc3d6..a31694f7f 100644 --- a/oc2/xxdr.c +++ b/oc2/xxdr.c @@ -59,6 +59,7 @@ #include #endif + #ifdef ENDIAN_VALIDATE #include #endif From b5697ae20f8558df160f0e1404e3a26ea2db3600 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Wed, 17 Apr 2013 18:58:37 +0000 Subject: [PATCH 2/2] Rebuild the projection merging code --- libdap2/constraints3.c | 29 ++--- libdap2/dceconstraints.c | 231 +++++++++++++++++++++++++++------------ libdap2/dceconstraints.h | 14 ++- libdap2/env | 4 +- ncdap_test/tst_remote.sh | 4 +- 5 files changed, 193 insertions(+), 89 deletions(-) diff --git a/libdap2/constraints3.c b/libdap2/constraints3.c index aa063d4f5..017a63dd8 100644 --- a/libdap2/constraints3.c +++ b/libdap2/constraints3.c @@ -71,11 +71,6 @@ mapconstraints3(DCEconstraint* constraint, proj->var->annotation = (void*)cdfmatch; } -#ifdef DEBUG -fprintf(stderr,"mapconstraint.projections: %s\n", - dumpprojections(dceprojections)); -#endif - done: return THROW(ncstat); } @@ -450,13 +445,15 @@ fprintf(stderr,"buildvaraprojection: skeleton: %s\n",dumpprojection(projection)) for(i=0;irank;j++) { + size_t count = 0; DCEslice* slice = &segment->slices[j]; /* make each slice represent the corresponding start/count/stride */ slice->first = startp[dimindex+j]; slice->stride = stridep[dimindex+j]; - slice->count = countp[dimindex+j]; - slice->length = slice->count * slice->stride; + count = countp[dimindex+j]; + slice->count = count; + slice->length = count * slice->stride; slice->stop = (slice->first + slice->length); if(slice->stop > slice->declsize) { slice->stop = slice->declsize; @@ -489,8 +486,9 @@ iswholeslice(DCEslice* slice, CDFnode* dim) if(dim != NULL) { if(slice->stop != dim->dim.declsize) return 0; } else if(dim == NULL) { + size_t count = slice->count; if(slice->declsize == 0 - || slice->count != slice->declsize) return 0; + || count != slice->declsize) return 0; } return 1; } @@ -707,9 +705,13 @@ slicematch(NClist* seglist1, NClist* seglist2) if(seg1->rank != seg2->rank) return 0; for(j=0;jrank;j++) { - if(seg1->slices[j].first != seg2->slices[j].first - || seg1->slices[j].count != seg2->slices[j].count - || seg1->slices[j].stride != seg2->slices[j].stride) + DCEslice* slice1 = &seg1->slices[j]; + DCEslice* slice2 = &seg2->slices[j]; + size_t count1 = slice1->count; + size_t count2 = slice2->count; + if(slice1->first != slice2->first + || count1 != count2 + || slice1->stride != slice2->stride) return 0; } } @@ -814,7 +816,8 @@ fprintf(stderr,"restrictprojection.before: constraints=|%s| vara=|%s|\n", result = (DCEprojection*)dceclone((DCEnode*)result); /* so we can modify */ #ifdef DEBUG1 -fprintf(stderr,"restrictprojection.choice: |%s|\n",dumpprojection(result)); +fprintf(stderr,"restrictprojection.choice: base=|%s| add=|%s|\n", + dumpprojection(result),dumpprojection(var)); #endif /* We need to merge the projection from the projection list with the var projection @@ -840,7 +843,7 @@ dapshiftslice(DCEslice* slice) slice->first = 0; slice->stride = 1; slice->length = slice->count; - slice->stop = slice->count; + slice->stop = slice->length; } int diff --git a/libdap2/dceconstraints.c b/libdap2/dceconstraints.c index 0b47142ad..505e71d14 100644 --- a/libdap2/dceconstraints.c +++ b/libdap2/dceconstraints.c @@ -63,97 +63,193 @@ fprintf(stderr,"constraint: %s",dcetostring((DCEnode*)dapconstraint)); } #endif -/* Worksheet +#ifdef DEBUG1 +static void +dumpslice(const char* prefix, DCEslice* s) +{ +#if 1 + int v = dceverbose; + dceverbose = 1; + fprintf(stderr,"%s: %s\n",prefix,dcetostring(s)); + dceverbose = v; +#else + size_t last = (s->first+s->length)-1; + fprintf(stderr,"%s: [%lu:%lu:%lu p=%lu l=%lu c=%lu]\n", + prefix,s->first,s->stride,last,s->stop,s->length,s->count); +#endif +} +#endif -mg.st = md.st * ms.st -mg.f = md.f+(ms.f*md.st) -mg.l = ((ms.l-1) / ms.st) * mg.st + 1 -mg.p = mg.f + mg.l -mg.c = mg.l / mg.st +/* +Logical derivation of s1 compose s2 = sr + +The composed value of first and stride are easy: +sr.first = s1.first + (s2.first * s1.stride) +sr.stride = s1.stride * s2.stride + +Finding sr.length, however, is significantly more +complex. In particular, the length specified by s2 +may be more or less than the number of values +provided by s1. + +One way to compute sr.length is as follows +. Compute the count of the number of values provided by s1 + count1 = (s1.length+(s1.stride-1))/s1.stride + Note that count1 is effectively the number of elements + available to s2. +. Compute the stop point of s1 using count1 and wrt original data + (stop point is first + length) + stop1 = (s1.first + (count1 * s1.stride)) +. Compute the number of elements resulting from applying s2 + count2 = (s2.length+(s2.stride-1))/s2.stride +. Compute the stop point of s2 using count2 and wrt s1 output + stop2 = (s2.first + (count2 * s2.stride)) +. Now we can convert stop2 back into a stop point wrt the original data + stopx = (s1.first + (stop2*s1.stride)) +. Make sure we do not overrun the output of s2 by using the min() + stopr = min(stopx,stop1) +. Compute the result length wrt the original data + sr.length = (stopr - sr.first) + +Example 1: +0000000000111111111 +0123456789012345678 + 0 1 2 3 4 s1=(f=1 st=2 len=9) + 0 1 2 3 4 s2=(f=0 st=1 len=5) + xxxxxxxxxx + 0 1 3 4 5 sr=(f=1 st=2 len=9) +sr.f = 1+(0*2) = 1 +sr.st = 2*1 = 2 +count1 = (9+(2-1))/2 = 5 +stop1 = (1+(5*2)) = 11 +count2 = (5+(1-1))/1 = 5 +stop2 = 0+(5*1) = 5 +stopx = (1+(5*2)) = 11 +stopr = min(11,11) = 11 +sr.l = (11 - 2) = 9 + +Example 2: 0000000000111111111122222222223 0123456789012345678901234567890 - xxxxxx - xxxxxx - 0 1 2 3 4 5 6 7 8 md=(st=3 f=1 l=25 p=26) - 0 1 2 ms=(st=2 f=3 l=5 p=8 ) - ---------------------------- - mg=(st=6 f=10 p=23 l=13) -c = 4 / 2 = 2 -l = 2 * 6 + 1 = 13 + 0 1 2 3 4 5 6 7 8 _ s1=(f=1 st=3 len=25) + 0 1 2 _ s2=(f=3 st=2 len=5) + xxxxxxxxxxxxxxxxxx + 0 1 2 sr=(f=10 st=6 l=18) +sr.f = 1+(3*3) = 10 +sr.st = 3*2 =6 +count1 = (25+(3-1))/3 = 9 +stop1 = (1+(9*3)) = 28 +count2 = (5+(2-1))/2 = 3 +stop2 = (3+(3*2)) = 9 +stopx = (1+(9*3)) = 28 +stopr = min(28,28) = 28 +sr.l = 28 - 10 = 18 + + +Example 3: 0000000000111111 0123456789012345 - 0 1 2 3 4 md=(st=2 f=1 l=9 p=10) - 0 1 2 ms=(st=1 f=2 l=3 p=5) - ---------------------------- - mg=(st=2 f=5 p=10 l=5 ) -c = 2/1 = 2 -l = 2 * 2 + 1 = 13 + 0 1 2 3 4 _ _ s1=(f=1 st=2 l=9) + 0 1 2 3 _ s2=(f=2 st=1 l=4) + xxxxxxp ---------------------------- + sr=(f=5 st=2 l=6) +sr.f = 1+(2*2) = 5 +sr.st = 2*1 = 2 +count1 = (9+(2-1))/2 = 5 +stop1 = 1+(5*2) = 11 +count2 = (4+(1-1))/1 = 4 +stop2 = 2+(4*1) = 6 +stopx = 1*(6*2) = 13 +stopr = min(13,11) = 11 +sr.l = 11 - 5 = 6 +Example 4: 0000000000111111111 0123456789012345678 - 0 1 2 3 4 5 6 7 8 md=(st=2 f=1 l=17 p=18) - 0 1 2 ms=(st=2 f=3 l=5 p=8) - ---------------------------- - mg=(st=4 f=7 p=16 l=9 ) -c = 4/2 = 2 -l = 2 * 4 + 1 = 9 + 0 1 2 3 4 _ _ s1=(f=1 st=2 l=9) + 0 1 p s2=(f=2 st=2 l=4) + xxxxxx ---------------------------- + sr=(f=5 st=4 l=6) +sr.f = 1+(2*2) = 5 +sr.st = 2*2 = 4 +count1 = (9+(2-1))/2 = 5 +stop1 = 1+(5*2) = 11 +count2 = (4+(2-1))/2 = 2 +stop2 = 2+(2*2) = 6 +stopx = 1*(6*2) = 13 +stopr = min(13,11) = 11 +sr.l = 11 - 5 = 6 -0000000000111111111 -0123456789012345678 - 0 1 2 3 4 md=(st=2 f=1 l=9 p=10) - 0 1 2 3 4 ms=(st=1 f=0 l=5 p=5) +Example 5: +00000000001 +01234567890 +012 s1=(f=0 st=1 l=3) +012 s2=(f=0 st=1 l=3) ---------------------------- - mg=(st=2 f=1 p=10 l=9 ) -c = 4/1 = 4 -l = 4 * 2 + 1 = 9 + sr=(f=0 st=1 l=3) +sr.f = 0+(0*1) = 0 +sr.st = 1*1 = 1 +count1 = (3+(1-1))/1 = 3 +stop1 = 0+(3*1) = 3 +count2 = (3+(1-1))/1 = 3 +stop2 = 0+(3*1) = 3 +stopx = 0+(3*1) = 3 +stopr = min(3,3) = 3 +sr.l = 3 - 0 = 3 00000 01234 -01 md=(st=1 f=0 l=2 p=2) -0 ms=(st=1 f=0 l=1 p=1) +01 s1=(f=0 st=1 l=2) +0 s2=(f=0 st=1 l=1) ---------------------------- - mg=(st=1 f=0 p=1 l=1 ) -c = 0/1 = 0 -l = 0 * 1 + 1 = 1 - -000000000011 -012345678901 -012 md=(st=1 f=0 l=3 p=3) -012 ms=(st=1 f=0 l=3 p=2) - ---------------------------- - mg=(st=1 f=0 p=3 l=3 ) -c = 2/1 = 2 -l = 2 * 1 + 1 = 3 + sr=(f=0 st=1 l=1) +sr.f = 0+(0*1) = 0 +sr.st = 1*1 = 1 +count1 = (2+(1-1))/1 = 2 +stop1 = 0+(2*1) = 2 +count2 = (1+(1-1))/1 = 1 +stop2 = 0+(1*1) = 1 +stopx = 0+(1*1) = 1 +stopr = min(1,2) = 1 +sr.l = 1 - 0 = 1 */ -/* Merge slice src into slice dst; dst != src */ +/* compose slice src into slice dst; dst != src. +Compose means that the src constraint is applied +to the output of the dst constraint. + */ int -dceslicemerge(DCEslice* dst, DCEslice* src) +dceslicecompose(DCEslice* s1, DCEslice* s2) { int err = NC_NOERR; DCEslice tmp; - + size_t count1, stop1, count2, stop2, stopx, stopr; +#ifdef DEBUG1 +dumpslice("compose: s1",s1); +dumpslice("compose: s2",s2); +#endif tmp.node.sort = CES_SLICE; - tmp.stride = (dst->stride * src->stride); - tmp.first = (dst->first+((src->first)*(dst->stride))); - tmp.length = (((src->length - 1) / src->stride) * tmp.stride) + 1; - tmp.stop = tmp.first + tmp.length; - tmp.count = tmp.length / tmp.stride; - /* use max declsize */ - if(dst->declsize > src->declsize) { - tmp.declsize = dst->declsize; - } else { - tmp.declsize = src->declsize; - } - if(tmp.length % tmp.stride != 0) tmp.count++; - if(tmp.first >= dst->stop || tmp.stop > dst->stop) - err = NC_EINVALCOORDS; - else - *dst = tmp; + tmp.first = s1->first+(s2->first * s1->stride); + tmp.stride = s1->stride * s2->stride; + count1 = (s1->length + (s1->stride - 1))/s1->stride; + stop1 = s1->first + (count1 * s1->stride); + count2 = (s2->length + (s2->stride - 1))/s2->stride; + stop2 = s2->first + (count2 * s2->stride); + stopx = s1->first + (stop2 * s1->stride); + stopr = (stopx < stop1 ? stopx : stop1); /* min(stopx,stop1) */ + tmp.length = (stopr - tmp.first); + tmp.declsize = (s1->declsize < s2->declsize ? s2->declsize : s1->declsize); /* use max declsize */ + /* fill in other fields */ + tmp.stop = tmp.first + tmp.length; + tmp.count = (tmp.length + (tmp.stride - 1))/tmp.stride; + *s1 = tmp; +#ifdef DEBUG1 +dumpslice("compose: result",&tmp); +#endif return err; } @@ -231,7 +327,7 @@ dcemergeprojections(DCEprojection* merged, DCEprojection* addition) /* If one segment has larger rank, then copy the extra slices unchanged */ for(j=0;jrank;j++) { if(j < mergedseg->rank) - dceslicemerge(mergedseg->slices+j,addedseg->slices+j); + dceslicecompose(mergedseg->slices+j,addedseg->slices+j); else mergedseg->slices[j] = addedseg->slices[j]; } @@ -1145,3 +1241,4 @@ dcedumprawlist(NClist* list, NCbytes* buf) } ncbytescat(buf,")"); } + diff --git a/libdap2/dceconstraints.h b/libdap2/dceconstraints.h index 1fb4982e0..c081cd3da 100644 --- a/libdap2/dceconstraints.h +++ b/libdap2/dceconstraints.h @@ -16,14 +16,16 @@ typedef struct DCEnode { CEsort sort; } DCEnode; -/* The slice structure is assumed common to all uses */ +/* The slice structure is assumed common to all uses. + It is initially established from [first:stride:last] +*/ typedef struct DCEslice { DCEnode node; - size_t first; - size_t count; - size_t length; /* count*stride */ + size_t first; size_t stride; - size_t stop; /* == first + count*/ + size_t length; + size_t stop; /* first + length */ + size_t count; /* (length + (stride-1))/ stride == actual # of elements returned to client*/ size_t declsize; /* from defining dimension, if any.*/ } DCEslice; @@ -89,7 +91,7 @@ typedef struct DCEconstraint { extern int dceparseconstraints(char* constraints, DCEconstraint* DCEonstraint); -extern int dceslicemerge(DCEslice* dst, DCEslice* src); +extern int dceslicecompose(DCEslice* dst, DCEslice* src); extern int dcemergeprojectionlists(NClist* dst, NClist* src); extern DCEnode* dceclone(DCEnode* node); diff --git a/libdap2/env b/libdap2/env index 5dc696879..c90ee1d04 100644 --- a/libdap2/env +++ b/libdap2/env @@ -10,8 +10,8 @@ PROG="$TOP/ncdump/ncdump" P=`pwd` -F="http://motherlode.ucar.edu:8081/dts/ingrid" -CON="v3H[0:0][0:43]" +F="http://test.opendap.org:8080/dods/dts/test.03" +CON="i32[0:1][1:2][0:2]" PARMS="[log]" #PARMS="${PARMS}[netcdf3]" diff --git a/ncdap_test/tst_remote.sh b/ncdap_test/tst_remote.sh index cd2dc3a01..2df7d871d 100755 --- a/ncdap_test/tst_remote.sh +++ b/ncdap_test/tst_remote.sh @@ -29,7 +29,9 @@ longtests="$5" if test "x$timing" = "x1" ; then leakcheck=0; fi # get the list of test files -WHICHTESTS="S1 C1 C2" +# Currently C2 fails because server is not responding +#WHICHTESTS="S1 C1 C2" +WHICHTESTS="S1 C1" if test -n "$longtests"; then WHICHTESTS="${WHICHTESTS} L1 LC1 LC2" fi