Merge pull request #2000 from DennisHeimbigner/badfilter.dmh

Improve error message when non-existent filter is encountered.

RELEASE_NOTES.md

@@ -6,6 +6,7 @@ Release Notes       {#RELEASE_NOTES}
 This file contains a high-level description of this package's evolution. Releases are in reverse chronological order (most recent first). Note that, as of netcdf 4.2, the `netcdf-c++` and `netcdf-fortran` libraries have been separated into their own libraries.
 ## 4.8.1 - TBD
 
+* [Enhancement] Improve the error reporting when attempting to use a filter for which no implementation can be found in HDF5_PLUGIN_PATH. See [Github #2000](https://github.com/Unidata/netcdf-c/pull/2000) for more information.
 * [Bug Fix] Fix `make distcheck` issue in `nczarr_test/` directory. See [Github #2007](https://github.com/Unidata/netcdf-c/issues/2007).
 * [Bug Fix] Fix bug in NCclosedir in dpathmgr.c. See [Github #2003](https://github.com/Unidata/netcdf-c/issues/2003).
 * [Bug Fix] Fix bug in ncdump that assumes that there is a relationship between the total number of dimensions and the max dimension id. See [Github #2004](https://github.com/Unidata/netcdf-c/issues/2004).  
@@ -20,7 +21,6 @@ This file contains a high-level description of this package's evolution. Release
 ## 4.8.0 - March 30, 2021
 
 * [Enhancement] Bump the NC_DISPATCH_VERSION from 2 to 3, and as a side effect, unify the definition of NC_DISPATCH_VERSION so it only needs to be defined in CMakeLists.txt and configure.ac. See [Github #1945](https://github.com/Unidata/netcdf-c/pull/1945) for more information.
->>>>>>> master
 * [Enhancement] Provide better cross platform path name management. This converts paths for various platforms (e.g. Windows, MSYS, etc.) so that they are in the proper format for the executing platform. See [Github #1958](https://github.com/Unidata/netcdf-c/pull/1958) for more information.
 * [Bug Fixes] The nccopy program was treating -d0 as turning deflation on rather than interpreting it as "turn off deflation". See [Github #1944](https://github.com/Unidata/netcdf-c/pull/1944) for more information.
 * [Enhancement] Add support for storing NCZarr data in zip files. See [Github #1942](https://github.com/Unidata/netcdf-c/pull/1942) for more information.

include/hdf5internal.h

@@ -95,6 +95,8 @@ typedef struct NC_HDF5_VAR_INFO
     HDF5_OBJID_T *dimscale_hdf5_objids;
     nc_bool_t dimscale;          /**< True if var is a dimscale. */
     nc_bool_t *dimscale_attached;  /**< Array of flags that are true if dimscale is attached for that dim index. */
+    int flags;
+#       define NC_HDF5_VAR_FILTER_MISSING 1 /* if any filter is missing */
 } NC_HDF5_VAR_INFO_T;
 
 /* Struct to hold HDF5-specific info for a field. */
@@ -186,15 +188,17 @@ int NC4_hdf5_inq_var_filter_info(int ncid, int varid, unsigned int filterid, siz
 /* The NC_VAR_INFO_T->filters field is an NClist of this struct */
 struct NC_HDF5_Filter {
     int flags;             /**< Flags describing state of this filter. */
+#       define NC_HDF5_FILTER_MISSING 1 /* Filter implementation is not accessible */
     unsigned int filterid; /**< ID for arbitrary filter. */
     size_t nparams;        /**< nparams for arbitrary filter. */
     unsigned int* params;  /**< Params for arbitrary filter. */
 };
 
 int NC4_hdf5_filter_remove(NC_VAR_INFO_T* var, unsigned int id);
 int NC4_hdf5_filter_lookup(NC_VAR_INFO_T* var, unsigned int id, struct NC_HDF5_Filter** fi);
-int NC4_hdf5_addfilter(NC_VAR_INFO_T* var, unsigned int id, size_t nparams, const unsigned int* params);
+int NC4_hdf5_addfilter(NC_VAR_INFO_T* var, unsigned int id, size_t nparams, const unsigned int* params, int flags);
 int NC4_hdf5_filter_freelist(NC_VAR_INFO_T* var);
+int NC4_hdf5_find_missing_filter(NC_VAR_INFO_T* var, unsigned int* idp);
 
 /* Support functions for provenance info (defined in nc4hdf.c) */
 extern int NC4_hdf5get_libversion(unsigned*,unsigned*,unsigned*);/*libsrc4/nc4hdf.c*/

libdispatch/derror.c

@@ -261,7 +261,7 @@ const char *nc_strerror(int ncerr1)
       case NC_EFILTER:
 	 return "NetCDF: Filter error: bad id or parameters";
       case NC_ENOFILTER:
-	 return "NetCDF: Filter error: filter not defined for variable";
+	 return "NetCDF: Filter error: unimplemented filter encountered";
       case NC_ECANTEXTEND:
 	return "NetCDF: Attempt to extend dataset during NC_INDEPENDENT I/O operation. Use nc_var_par_access to set mode NC_COLLECTIVE before extending variable.";
       case NC_EMPI: return "NetCDF: MPI operation failed.";

libhdf5/hdf5filter.c

@@ -248,7 +248,7 @@ PRINTFILTER(spec,"free");
 }
 
 int
-NC4_hdf5_addfilter(NC_VAR_INFO_T* var, unsigned int id, size_t nparams, const unsigned int* params)
+NC4_hdf5_addfilter(NC_VAR_INFO_T* var, unsigned int id, size_t nparams, const unsigned int* params, int flags)
 {
     int stat = NC_NOERR;
     struct NC_HDF5_Filter* fi = NULL;
@@ -280,6 +280,7 @@ NC4_hdf5_addfilter(NC_VAR_INFO_T* var, unsigned int id, size_t nparams, const un
 	    {stat = NC_ENOMEM; goto done;}
         memcpy(fi->params,params,sizeof(unsigned int)*fi->nparams);
     }
+    fi->flags = flags;
     if(!olddef) {
         nclistpush(flist,fi);
 PRINTFILTERLIST(var,"add");
@@ -346,6 +347,8 @@ NC4_hdf5_def_var_filter(int ncid, int varid, unsigned int id, size_t nparams,
     NC_GRP_INFO_T* grp = NULL;
     NC_VAR_INFO_T* var = NULL;
     struct NC_HDF5_Filter* oldspec = NULL;
+    int flags = 0;
+    htri_t avail = -1;
 #ifdef HAVE_H5Z_SZIP
     int havedeflate = 0;
     int haveszip = 0;
@@ -398,6 +401,16 @@ NC4_hdf5_def_var_filter(int ncid, int varid, unsigned int id, size_t nparams,
 	}
 #endif /* HAVE_H5Z_SZIP */
 
+	/* See if this filter is missing or not */
+	if((avail = H5Zfilter_avail(id)) < 0)
+ 	    {stat = NC_EHDFERR; goto done;} /* Something in HDF5 went wrong */
+	if(!avail) {
+            NC_HDF5_VAR_INFO_T* hdf5_var = (NC_HDF5_VAR_INFO_T *)var->format_var_info;
+	    flags |= NC_HDF5_FILTER_MISSING;
+	    /* mark variable as unreadable */
+	    hdf5_var->flags |= NC_HDF5_VAR_FILTER_MISSING;
+	}
+
 	/* If incoming filter not already defined, then check for conflicts */
 	if(oldspec == NULL) {
             if(id == H5Z_FILTER_DEFLATE) {
@@ -454,7 +467,7 @@ NC4_hdf5_def_var_filter(int ncid, int varid, unsigned int id, size_t nparams,
         }
 #endif
 	/* addfilter can handle case where filter is already defined, and will just replace parameters */
-        if((stat = NC4_hdf5_addfilter(var,id,nparams,params)))
+        if((stat = NC4_hdf5_addfilter(var,id,nparams,params,flags)))
                 goto done;
 #ifdef USE_PARALLEL
 #ifdef HDF5_SUPPORTS_PAR_FILTERS
@@ -542,3 +555,19 @@ NC4_hdf5_inq_var_filter_info(int ncid, int varid, unsigned int id, size_t* npara
     return stat;
 
 }
+
+/* Return ID for the first missing filter; 0 if no missing filters */
+int
+NC4_hdf5_find_missing_filter(NC_VAR_INFO_T* var, unsigned int* idp)
+{
+    int i,stat = NC_NOERR;
+    NClist* flist = (NClist*)var->filters;
+    int id = 0;
+
+    for(i=0;i<nclistlength(flist);i++) {
+	struct NC_HDF5_Filter* spec = (struct NC_HDF5_Filter*)nclistget(flist,i);
+	if(spec->flags & NC_HDF5_FILTER_MISSING) {id = spec->filterid; break;}
+    }
+    if(idp) *idp = id;
+    return stat;
+}

libhdf5/hdf5open.c

@@ -1013,22 +1013,35 @@ static int get_filter_info(hid_t propid, NC_VAR_INFO_T *var)
     size_t cd_nelems;
     int f;
     int stat = NC_NOERR;
+    NC_HDF5_VAR_INFO_T *hdf5_var;
 
     assert(var);
 
+    /* Get HDF5-sepecific var info. */
+    hdf5_var = (NC_HDF5_VAR_INFO_T *)var->format_var_info;
+
     if ((num_filters = H5Pget_nfilters(propid)) < 0)
 	{stat = NC_EHDFERR; goto done;}
 
     for (f = 0; f < num_filters; f++)
     {
+	int flags = 0;
+	htri_t avail = -1;
 	cd_nelems = 0;
         if ((filter = H5Pget_filter2(propid, f, NULL, &cd_nelems, NULL, 0, NULL, NULL)) < 0)
-	    {stat = NC_EHDFERR; goto done;}
+ 	    {stat = NC_ENOFILTER; goto done;} /* Assume this means an unknown filter */
+	if((avail = H5Zfilter_avail(filter)) < 0)
+ 	    {stat = NC_EHDFERR; goto done;} /* Something in HDF5 went wrong */
+	if(!avail) {
+	    flags |= NC_HDF5_FILTER_MISSING;
+	    /* mark variable as unreadable */
+	    hdf5_var->flags |= NC_HDF5_VAR_FILTER_MISSING;
+	}
 	if((cd_values = calloc(sizeof(unsigned int),cd_nelems))==NULL)
-	    {stat = NC_EHDFERR; goto done;}
+ 	    {stat = NC_ENOMEM; goto done;}
         if ((filter = H5Pget_filter2(propid, f, NULL, &cd_nelems, cd_values, 0, NULL, NULL)) < 0)
-	    {stat = NC_EHDFERR; goto done;}
-        switch (filter)
+ 	    {stat = NC_EHDFERR; goto done;} /* Something in HDF5 went wrong */
+	switch (filter)
         {
         case H5Z_FILTER_SHUFFLE:
             var->shuffle = NC_TRUE;
@@ -1042,15 +1055,15 @@ static int get_filter_info(hid_t propid, NC_VAR_INFO_T *var)
             if (cd_nelems != CD_NELEMS_ZLIB ||
                 cd_values[0] > NC_MAX_DEFLATE_LEVEL)
 		    {stat = NC_EHDFERR; goto done;}
-	    if((stat = NC4_hdf5_addfilter(var,filter,cd_nelems,cd_values)))
+	    if((stat = NC4_hdf5_addfilter(var,filter,cd_nelems,cd_values,flags)))
 	       goto done;
             break;
 
         case H5Z_FILTER_SZIP: {
             /* Szip is tricky because the filter code expands the set of parameters from 2 to 4
                and changes some of the parameter values; try to compensate */
             if(cd_nelems == 0) {
-		if((stat = NC4_hdf5_addfilter(var,filter,0,NULL)))
+		if((stat = NC4_hdf5_addfilter(var,filter,0,NULL,flags)))
 		   goto done;
             } else {
                 /* fix up the parameters and the #params */
@@ -1060,16 +1073,16 @@ static int get_filter_info(hid_t propid, NC_VAR_INFO_T *var)
 		/* Fix up changed params */
 		cd_values[0] &= (H5_SZIP_ALL_MASKS);
 		/* Save info */
-		stat = NC4_hdf5_addfilter(var,filter,cd_nelems,cd_values);
+		stat = NC4_hdf5_addfilter(var,filter,cd_nelems,cd_values,flags);
 		if(stat) goto done;
             }
             } break;
 
         default:
             if(cd_nelems == 0) {
-  	        if((stat = NC4_hdf5_addfilter(var,filter,0,NULL))) goto done;
+  	        if((stat = NC4_hdf5_addfilter(var,filter,0,NULL,flags))) goto done;
             } else {
-  	        stat = NC4_hdf5_addfilter(var,filter,cd_nelems,cd_values);
+  	        stat = NC4_hdf5_addfilter(var,filter,cd_nelems,cd_values,flags);
 		if(stat) goto done;
             }
             break;

libhdf5/hdf5var.c

@@ -1375,6 +1375,14 @@ NC4_put_vars(int ncid, int varid, const size_t *startp, const size_t *countp,
         return retval;
     assert(hdf5_var->hdf_datasetid && (!var->ndims || (startp && countp)));
 
+    /* Verify that all the variable's filters are available */
+    if(hdf5_var->flags & NC_HDF5_VAR_FILTER_MISSING) {
+	unsigned id = 0;
+	NC4_hdf5_find_missing_filter(var, &id);
+	LOG((0,"missing filter: variable=%s id=%u",var->hdr.name,id));
+        return NC_ENOFILTER;
+    }
+
     /* Convert from size_t and ptrdiff_t to hssize_t, and hsize_t. */
     /* Also do sanity checks */
     for (i = 0; i < var->ndims; i++)
@@ -1693,6 +1701,14 @@ NC4_get_vars(int ncid, int varid, const size_t *startp, const size_t *countp,
         return retval;
     assert(hdf5_var->hdf_datasetid && (!var->ndims || (startp && countp)));
 
+    /* Verify that all the variable's filters are available */
+    if(hdf5_var->flags & NC_HDF5_VAR_FILTER_MISSING) {
+	unsigned id = 0;
+	NC4_hdf5_find_missing_filter(var, &id);
+	LOG((0,"missing filter: variable=%s id=%u",var->hdr.name,id));
+        return NC_ENOFILTER;
+    }
+
     /* Convert from size_t and ptrdiff_t to hsize_t. Also do sanity
      * checks. */
     for (i = 0; i < var->ndims; i++)

libnczarr/zinternal.h

@@ -225,6 +225,7 @@ int ncz_find_default_chunksizes2(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var);
 /* The NC_VAR_INFO_T->filters field is an NClist of this struct */
 struct NCZ_Filter {
     int flags;             /**< Flags describing state of this filter. */
+#define NCZ_FILTER_MISSING 1 /* Signal filter implementation is not available */
     unsigned int filterid; /**< ID for arbitrary filter. */
     size_t nparams;        /**< nparams for arbitrary filter. */
     unsigned int* params;  /**< Params for arbitrary filter. */

nc_test4/tst_filter.sh

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 if test "x$srcdir" = x ; then srcdir=`pwd`; fi
 . ../test_common.sh
@@ -41,10 +41,23 @@ trimleft() {
 sed -e 's/[ 	]*\([^ 	].*\)/\1/' <$1 >$2
 }
 
+# Hide/unhide the bzip2 filter
+hidebzip2() {
+  rm -fr ${HDF5_PLUGIN_PATH}/save
+  mkdir ${HDF5_PLUGIN_PATH}/save
+  mv ${BZIP2PATH} ${HDF5_PLUGIN_PATH}/save
+}
+
+unhidebzip2() {
+  mv ${HDF5_PLUGIN_PATH}/save/${BZIP2LIB} ${HDF5_PLUGIN_PATH}
+  rm -fr ${HDF5_PLUGIN_PATH}/save
+}
+
 # Locate the plugin path and the library names; argument order is critical
 # Find bzip2 and capture
 findplugin h5bzip2
-BZIP2PATH="${HDF5_PLUGIN_PATH}/${HDF5_PLUGIN_LIB}"
+BZIP2LIB="${HDF5_PLUGIN_LIB}"
+BZIP2PATH="${HDF5_PLUGIN_PATH}/${BZIP2LIB}"
 # Find misc and capture
 findplugin misc
 MISCPATH="${HDF5_PLUGIN_PATH}/${HDF5_PLUGIN_LIB}"
@@ -169,23 +182,31 @@ fi
 if test "x$UNK" = x1 ; then
 echo "*** Testing access to filter info when filter dll is not available"
 rm -f bzip2.nc ./tst_filter.txt
-# build bzip2.nc
+# xfail build bzip2.nc 
+hidebzip2
+if ${NCGEN} -lb -4 -o bzip2.nc ${srcdir}/bzip2.cdl ; then
+    echo "*** FAIL: ncgen"
+else
+    echo "*** XFAIL: ncgen"
+fi
+unhidebzip2    
+# build bzip2.nc 
 ${NCGEN} -lb -4 -o bzip2.nc ${srcdir}/bzip2.cdl
-# dump and clean bzip2.nc header only when filter is avail
-${NCDUMP} -hs bzip2.nc > ./tst_filter.txt
-# Remove irrelevant -s output
-sclean ./tst_filter.txt bzip2.dump
 # Now hide the filter code
-mv ${BZIP2PATH} ${BZIP2PATH}.save
-# dump and clean bzip2.nc header only when filter is not avail
+hidebzip2
 rm -f ./tst_filter.txt
-${NCDUMP} -hs bzip2.nc > ./tst_filter.txt
-# Remove irrelevant -s output
-sclean ./tst_filter.txt bzip2x.dump
+# This will xfail
+if ${NCDUMP} -s bzip2.nc > ./tst_filter.txt ; then
+    echo "*** FAIL: ncdump -hs bzip2.nc"
+else
+    echo "*** XFAIL: ncdump -hs bzip2.nc"
+fi
 # Restore the filter code
-mv ${BZIP2PATH}.save ${BZIP2PATH}
-diff -b -w ./bzip2.dump ./bzip2x.dump
-echo "*** Pass: ncgen dynamic filter"
+unhidebzip2
+# Verify we can see filter when using -h
+rm -f ./tst_filter.txt
+${NCDUMP} -hs bzip2.nc > ./tst_filter.txt
+echo "*** Pass: unknown filter"
 fi
 
 if test "x$NGC" = x1 ; then