Merge pull request #3201 from DennisHeimbigner/ncdumpfixes.dmh

WardF · web-flow · commit 4a8e004fe4d0 · 2025-12-02T13:14:50.000-07:00
Fix Issue with Numcodecs encoding problems
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.10.0 - TBD
 
+* Fix Issue with Numcodecs encoding problems where integer filter parameters are being encoded as strings. See [Github 3201](https://github.com/Unidata/netcdf-c/issues/3201) for more information. 
 * Clean up minor problems with DAP2/DAP4 code. See [Github #3215](https://github.com/Unidata/netcdf-c/pull/3215) for more information.
 * Cleanup RELEASE_NOTES.md. See [Github 3190](https://github.com/Unidata/netcdf-c/issues/3190) for more information. 
 * Rebuild the S3-related code and other changes necessary to build cleanly on github actions. See [Github #3159](https://github.com/Unidata/netcdf-c/pull/3159) for more information.
diff --git a/dap4_test/Makefile.am b/dap4_test/Makefile.am
@@ -8,10 +8,10 @@ include $(top_srcdir)/lib_flags.am
 LDADD = ${top_builddir}/liblib/libnetcdf.la
 
 # Un comment to use a more verbose test driver
-#SH_LOG_DRIVER = $(SHELL) $(top_srcdir)/test-driver-verbose
-#LOG_DRIVER = $(SHELL) $(top_srcdir)/test-driver-verbose
-#TEST_LOG_DRIVER = $(SHELL) $(top_srcdir)/test-driver-verbose
-#TESTS_ENVIRONMENT = export SETX=1;
+SH_LOG_DRIVER = $(SHELL) $(top_srcdir)/test-driver-verbose
+LOG_DRIVER = $(SHELL) $(top_srcdir)/test-driver-verbose
+TEST_LOG_DRIVER = $(SHELL) $(top_srcdir)/test-driver-verbose
+TESTS_ENVIRONMENT = export SETX=1;
 
 # Note which tests depend on other tests. Necessary for make -j check.
 TEST_EXTENSIONS = .sh
diff --git a/dap4_test/test_data.sh b/dap4_test/test_data.sh
@@ -3,6 +3,7 @@
 if test "x$srcdir" = x ; then srcdir=`pwd`; fi
 . ../test_common.sh
 
+set -x
 set -e
 
 . ${top_srcdir}/dap4_test/d4test_common.sh
diff --git a/include/ncconfigure.h b/include/ncconfigure.h
@@ -98,6 +98,12 @@ unsigned long long int strtoull(const char*, char**, int);
 #endif
 #endif
 
+#ifndef HAVE_MEMMOVE
+#ifndef memmove
+void* memmove(void *dest, const void *src, size_t count);
+#endif
+#endif
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/libdap4/d4bytes.c b/libdap4/d4bytes.c
@@ -67,7 +67,7 @@ d4bytesconcat(D4bytes* dst, D4bytes* src)
 {
     if(dst == NULL && src != NULL && src->used > 0) {
 	void* target = d4bytesalloc(dst,src->used);
-	d4memmove(target,src->memory,src->used);
+	memmove(target,src->memory,src->used);
     }
     return dst;
 }
diff --git a/libdap4/d4chunk.c b/libdap4/d4chunk.c
@@ -118,7 +118,7 @@ NCD4_dechunk(NCD4response* resp)
             return processerrchunk(resp, (void*)pchunk, hdr.count);
         /* data chunk; possibly last; possibly empty */
         if(hdr.count > 0)
-            d4memmove(pappend,pchunk,hdr.count); /* overwrite the header; this the heart of dechunking */
+            memmove(pappend,pchunk,hdr.count); /* overwrite the header; this the heart of dechunking */
 	pappend += hdr.count; /* next append point */
         phdr = pchunk + hdr.count; /* point to header of next chunk */
         if(hdr.flags & NCD4_LAST_CHUNK) break;
diff --git a/libdap4/d4meta.c b/libdap4/d4meta.c
@@ -607,7 +607,7 @@ savevarbyid(NCD4node* group, NCD4node* var)
 {
     if(group->group.varbyid == NULL)
         group->group.varbyid = nclistnew();
-    nclistsetalloc(group->group.varbyid, (size_t)var->meta.id);
+    nclistsetlength(group->group.varbyid, (size_t)var->meta.id+1);
     nclistinsert(group->group.varbyid, (size_t)var->meta.id,var);
 }
 
diff --git a/libdap4/d4util.h b/libdap4/d4util.h
@@ -6,12 +6,6 @@
 #ifndef D4UTIL_H
 #define D4UTIL_H 1
 
-#ifdef HAVE_MEMMOVE
-#define d4memmove(dst,src,n) memmove(dst,src,n)
-#else
-#define d4memmove(dst,src,n) localmemmove(dst,src,n)
-#endif
-
 /* This is intended to be big enough to work as
    an offset/position/size for a file or a memory block.
 */
diff --git a/libdispatch/dmissing.c b/libdispatch/dmissing.c
@@ -160,3 +160,31 @@ NC_strcasestr(const char *s, const char *find)
     return ((char *)s);
 }
 #endif
+
+#ifndef HAVE_MEMMOVE
+/**
+Define an implementation of memmove
+in the event that it is not available.
+@param dst target of the move
+@param src source of the move
+@param count number of bytes to move.
+Note: dst and src can overlap
+*/
+void*
+memmove(void *dst, const void *src, size_t count)
+{
+    char *d = dst;
+    const char *s = src;
+
+    if (d < s) {
+        while (count--) {*d++ = *s++;}
+    } else {
+        d += count; /* Point to one past the end of destination */
+        s += count; /* Point to one past the end of source */
+        while (count--) {*(--d) = *(--s);} /* Decrement pointers and copy */
+    }
+    return dst;
+}
+
+#endif /*HAVE_MEMMOVE*/
+
diff --git a/libdispatch/ds3util.c b/libdispatch/ds3util.c
@@ -178,20 +178,20 @@ NC_s3urlrebuild(NCURI* url, NCS3INFO* s3, NCURI** newurlp)
 	/* bucket is unknown at this point */
 	svc = NCS3GS;
     } else { /* Presume Formats (8),(9),(10) */
-		if (nclistlength(hostsegments) > 3 && strcasecmp(nclistget(hostsegments, 1), "s3") == 0){
-			bucket = nclistremove(hostsegments, 0);
-			region = nclistremove(hostsegments, 2);
-			host = strdup(url->host + sizeof(bucket) + 1);
-		}else{
-			if (nclistlength(hostsegments) > 2 && strcasecmp(nclistget(hostsegments, 0), "s3") == 0){
-				region = nclistremove(hostsegments, 1);
-			}
-			if ((host = strdup(url->host)) == NULL){
-				stat = NC_ENOMEM;
-				goto done;
-			}
+	if (nclistlength(hostsegments) > 3 && strcasecmp(nclistget(hostsegments, 1), "s3") == 0){
+		bucket = nclistremove(hostsegments, 0);
+		region = nclistremove(hostsegments, 2);
+		host = strdup(url->host + sizeof(bucket) + 1);
+	}else{
+		if (nclistlength(hostsegments) > 2 && strcasecmp(nclistget(hostsegments, 0), "s3") == 0){
+			region = nclistremove(hostsegments, 1);
+		}
+		if ((host = strdup(url->host)) == NULL){
+			stat = NC_ENOMEM;
+			goto done;
 		}
 	}
+    }
 
     /* region = (1) from url, (2) s3->region, (3) default */
     if(region == NULL && s3 != NULL)
diff --git a/libdispatch/nclist.c b/libdispatch/nclist.c
@@ -7,12 +7,12 @@
 
 #include "nclist.h"
 
+#undef HAVE_MEMMOVE
+
 #if defined(_WIN32) && !defined(__MINGW32__)
 #define strcasecmp _stricmp
 #endif
 
-int nclistisnull(void* e) {return e == NULL;}
-
 #define NCLISTDEBUG 1
 
 #ifndef TRUE
@@ -25,6 +25,11 @@ int nclistisnull(void* e) {return e == NULL;}
 #define DEFAULTALLOC 16
 #define ALLOCINCR 16
 
+/*Forward */
+static void memmovex(void* dst, void* src, size_t len);
+
+int nclistisnull(void* e) {return e == NULL;}
+
 static int
 nclistfail(void)
 {
@@ -125,7 +130,8 @@ int
 nclistsetlength(NClist* l, size_t newlen)
 {
   if(l == NULL) return nclistfail();
-  if(newlen > l->alloc && !nclistsetalloc(l,newlen)) return nclistfail();
+  if(newlen >= l->alloc && !nclistsetalloc(l,newlen+1)) /* +1 in case newlen == l->alloc */
+      return nclistfail();
   if(newlen > l->length) {
       /* clear any extension */
       memset(&l->content[l->length],0,(newlen - l->length)*sizeof(void*));
@@ -167,7 +173,10 @@ nclistinsert(NClist* l, size_t index, void* elem)
   if(index > l->length) return nclistfail();
   nclistsetalloc(l,0);
   if(l->length > 0) {
+    memmovex(l->content+(index+1),l->content+index,(l->length-index)*sizeof(void*));
+#if 0
     for(i=l->length;i>index;i--) l->content[i] = l->content[i-1];
+#endif
   }
   l->content[index] = elem;
   l->length++;
@@ -212,7 +221,10 @@ nclistremove(NClist* l, size_t i)
   if((len=l->length) == 0) return NULL;
   if(i >= len) return NULL;
   elem = l->content[i];
+  memmovex(l->content+i,l->content+(i+1),(len-(i+1))*sizeof(void*));
+#if 0  
   for(i+=1;i<len;i++) l->content[i-1] = l->content[i];
+#endif
   l->length--;
   return elem;
 }
@@ -257,10 +269,9 @@ nclistelemremove(NClist* l, void* elem)
   for(i=0;i<nclistlength(l);i++) {
     void* candidate = l->content[i];
     if(elem == candidate) {
-      for(i+=1;i<len;i++) l->content[i-1] = l->content[i];
-      l->length--;
-      found = 1;
-      break;
+	nclistremove(l,i);
+        found = 1;
+        break;
     }
   }
   return found;
@@ -357,3 +368,35 @@ nclistnull(NClist* l)
     return 1;
 }
 
+/**************************************************/
+/* Utility functions */
+
+/**
+Define an internal form of memmove if not
+defined by platform.
+@param dst where to store bytes
+@param src source of bytes
+@param len no. of bytes to move
+@return void
+*/
+static void
+memmovex(void* dst, void* src, size_t len)
+{
+    if(len == 0) return;
+#ifdef HAVE_MEMMOVE
+    memmove(dst,src,len*sizeof(char));
+#else
+    {
+        char *d = dst;
+        const char *s = src;
+        if (d < s) {
+            while (len--) {*d++ = *s++;}
+        } else {
+            d += len; /* Point to one past the end of destination */
+            s += len; /* Point to one past the end of source */
+            while (len--) {*(--d) = *(--s);} /* Decrement pointers and copy */
+        }
+    }
+#endif
+}
+    
diff --git a/libhdf5/hdf5filter.c b/libhdf5/hdf5filter.c
@@ -100,9 +100,10 @@ NC4_hdf5_filter_freelist(NC_VAR_INFO_T* var)
 
     if(filters == NULL) goto done;
 PRINTFILTERLIST(var,"free: before");
-    /* Free the filter list backward */
-    for(size_t i = nclistlength(filters);i-->0;) {
-	struct NC_HDF5_Filter* spec = (struct NC_HDF5_Filter*)nclistremove(filters,i);
+    /* Free the filter list leaving ptrs in place */
+    for(size_t i = 0; i < nclistlength(filters);i++) {
+	struct NC_HDF5_Filter* spec = (struct NC_HDF5_Filter*)nclistget(filters,i);
+	assert((spec->nparams == 0 && spec->params == NULL) || (spec->nparams > 0 && spec->params != NULL));
 	if(spec->nparams > 0) nullfree(spec->params);
 	nullfree(spec);
     }
diff --git a/nc_test4/tst_specific_filters.sh b/nc_test4/tst_specific_filters.sh
@@ -134,7 +134,7 @@ testshuffle() {
 testdeflate() {
   zext=$1
   if ! avail deflate; then return 0; fi
-  runfilter $zext deflate '1,9' '[{\"id\": \"zlib\",\"level\": \"9\"}]'
+  runfilter $zext deflate '1,9' '[{\"id\": \"zlib\",\"level\": 9}]'
   # need to replace _DeflateLevel
 #  sed -e 's/_DeflateLevel = 9/_Filter = "1,9"/' < tmp_filt_deflate.dump > tmp_filt_deflatex.dump
   diff -b -w "tmp_filt_deflate.cdl" "tmp_filt_deflate.dump"
@@ -143,7 +143,7 @@ testdeflate() {
 testbzip2() {
   zext=$1
   if ! avail bzip2; then return 0; fi
-  runfilter $zext bzip2 '307,9' '[{\"id\": \"bz2\",\"level\": \"9\"}]'
+  runfilter $zext bzip2 '307,9' '[{\"id\": \"bz2\",\"level\": 9}]'
   diff -b -w "tmp_filt_bzip2.cdl" "tmp_filt_bzip2.dump"
 }
 
@@ -169,7 +169,7 @@ testblosc() {
 testzstd() {
   zext=$1
   if ! avail zstd; then return 0; fi
-  runfilter $zext zstd '32015,1' '[{\"id\": \"zstd\",\"level\": \"1\"}]'
+  runfilter $zext zstd '32015,1' '[{\"id\": \"zstd\",\"level\": 1}]'
   diff -b -w "tmp_filt_zstd.cdl" "tmp_filt_zstd.dump"
 }
 
diff --git a/nczarr_test/ref_bzip2.cdl b/nczarr_test/ref_bzip2.cdl
@@ -9,7 +9,7 @@ variables:
 		var:_Storage = "chunked" ;
 		var:_ChunkSizes = 4, 4, 4, 4 ;
 		var:_Filter = "307,9" ;
-		var:_Codecs = "[{\"id\": \"bz2\",\"level\": \"9\"}]" ;
+		var:_Codecs = "[{\"id\": \"bz2\",\"level\": 9}]" ;
 		var:_NoFill = "true" ;
 
 // global attributes:
diff --git a/nczarr_test/ref_filtered.cdl b/nczarr_test/ref_filtered.cdl
@@ -14,7 +14,7 @@ group: g {
   		var:_Storage = "chunked" ;
   		var:_ChunkSizes = 4, 4, 4, 4 ;
   		var:_Filter = "307,9" ;
-  		var:_Codecs = "[{\"id\": \"bz2\",\"level\": \"9\"}]" ;
+  		var:_Codecs = "[{\"id\": \"bz2\",\"level\": 9}]" ;
   		var:_NoFill = "true" ;
 
   // group attributes:
diff --git a/nczarr_test/ref_multi.cdl b/nczarr_test/ref_multi.cdl
@@ -9,7 +9,7 @@ variables:
 		var:_Storage = "chunked" ;
 		var:_ChunkSizes = 4, 4, 4, 4 ;
 		var:_Filter = "307,9|1,2|40000" ;
-		var:_Codecs = "[{\"id\": \"bz2\",\"level\": \"9\"},{\"id\": \"zlib\",\"level\": \"2\"},{\"id\": \"noop\"}]" ;
+		var:_Codecs = "[{\"id\": \"bz2\",\"level\": 9},{\"id\": \"zlib\",\"level\": 2},{\"id\": \"noop\"}]" ;
 		var:_NoFill = "true" ;
 
 // global attributes:
diff --git a/nczarr_test/run_corrupt.sh b/nczarr_test/run_corrupt.sh
@@ -18,6 +18,7 @@ export NCLOGGING=WARN
 
 testnoshape1() {
   zext=file
+  rm -fr ./ref_noshape.file
   unzip ${srcdir}/ref_noshape.file.zip
   fileargs ${ISOPATH}/ref_noshape "mode=zarr,$zext"
   rm -f tmp_noshape1_${zext}.cdl
diff --git a/nczarr_test/run_interop.sh b/nczarr_test/run_interop.sh
@@ -64,7 +64,9 @@ zext=$1
 case "$zext" in 
     file)
 	# need to unpack
+	rm -fr ./ref_power_901_constants
 	unzip ref_power_901_constants.zip >> tmp_ignore.txt
+	rm -fr ${ISOPATH}/ref_power_901_constants.file
 	mv ${ISOPATH}/ref_power_901_constants ${ISOPATH}/ref_power_901_constants.file
 	testcasefile power_901_constants zarr; # test xarray as default
 	;;
@@ -78,7 +80,9 @@ case "$zext" in
 	# Read a test case created by netcdf-java zarr.
 	# unpack
 	# Use gunzip because it always appears to be available
+	rm -fr ./ref_zarr_test_data.cdl
         gunzip -c ${srcdir}/ref_zarr_test_data.cdl.gz > ${ISOPATH}/ref_zarr_test_data.cdl
+	rm -fr ./ref_zarr_test_data_2d.cdl
         gunzip -c ${srcdir}/ref_zarr_test_data_2d.cdl.gz > ${ISOPATH}/ref_zarr_test_data_2d.cdl
         testcases3 zarr_test_data xarray
  	;;
diff --git a/nczarr_test/run_notzarr.sh b/nczarr_test/run_notzarr.sh
@@ -26,7 +26,9 @@ testfailed() {
 }
 
 # Make test sets
+rm -fr ./ref_notzarr.tar.gz
 cp ${srcdir}/ref_notzarr.tar.gz .
+rm -fr ./ref_notzarr.tar
 gunzip ref_notzarr.tar.gz
 tar -xf ref_notzarr.tar
 if test "x$FEATURE_S3TESTS" = xyes ; then
diff --git a/nczarr_test/run_oldkeys.sh b/nczarr_test/run_oldkeys.sh
@@ -35,6 +35,7 @@ rm -fr ref_oldkeys.file
 #echo "=================================="
 #echo "End Debug Stanza"
 #echo "=================================="
+rm -fr ./ref_oldkeys.file
 unzip ${srcdir}/ref_oldkeys.file.zip >> tmp_ignore.txt
 ${NCDUMP} $fileurl > tmp_oldkeys_${zext}.cdl
 ${ZMD} -t int $fileurl > tmp_oldkeys_${zext}.zmap
diff --git a/plugins/NCZhdf5filters.c b/plugins/NCZhdf5filters.c
@@ -87,7 +87,7 @@ NCZ_shuffle_hdf5_to_codec(size_t nparams, const unsigned* params, char** codecp)
     
     if(nparams > 0)
         typesize = params[0];
-    snprintf(json,sizeof(json),"{\"id\": \"%s\", \"elementsize\": \"%u\"}",NCZ_shuffle_codec.codecid,typesize);
+    snprintf(json,sizeof(json),"{\"id\": \"%s\", \"elementsize\": %u}",NCZ_shuffle_codec.codecid,typesize);
     if(codecp) {
         if((*codecp = strdup(json))==NULL) {stat = NC_ENOMEM; goto done;}
     }
@@ -300,10 +300,10 @@ NCZ_deflate_hdf5_to_codec(size_t nparams, const unsigned* params, char** codecp)
     char json[1024];
 
     if(nparams == 0 || params == NULL)
-        {stat = NC_EFILTER; goto done;}
+	{stat = NC_EFILTER; goto done;}
 
     level = params[0];
-    snprintf(json,sizeof(json),"{\"id\": \"%s\", \"level\": \"%u\"}",NCZ_zlib_codec.codecid,level);
+    snprintf(json,sizeof(json),"{\"id\": \"%s\", \"level\": %u}",NCZ_zlib_codec.codecid,level);
     if(codecp) {
         if((*codecp = strdup(json))==NULL) {stat = NC_ENOMEM; goto done;}
     }
diff --git a/plugins/NCZstdfilters.c b/plugins/NCZstdfilters.c
diff --git a/unit_test/Makefile.am b/unit_test/Makefile.am

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ d4bytesconcat(D4bytes* dst, D4bytes* src)`
`67`	`67`	`{`
`68`	`68`	`if(dst == NULL && src != NULL && src->used > 0) {`
`69`	`69`	`void* target = d4bytesalloc(dst,src->used);`
`70`		`- d4memmove(target,src->memory,src->used);`
	`70`	`+ memmove(target,src->memory,src->used);`
`71`	`71`	`}`
`72`	`72`	`return dst;`
`73`	`73`	`}`
Original file line number	Diff line number	Diff line change
`@@ -607,7 +607,7 @@ savevarbyid(NCD4node* group, NCD4node* var)`
`607`	`607`	`{`
`608`	`608`	`if(group->group.varbyid == NULL)`
`609`	`609`	`group->group.varbyid = nclistnew();`
`610`		`- nclistsetalloc(group->group.varbyid, (size_t)var->meta.id);`
	`610`	`+ nclistsetlength(group->group.varbyid, (size_t)var->meta.id+1);`
`611`	`611`	`nclistinsert(group->group.varbyid, (size_t)var->meta.id,var);`
`612`	`612`	`}`
`613`	`613`