Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message when non-existent filter is encountered. #2000

Merged
merged 4 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion include/hdf5internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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*/
Expand Down
2 changes: 1 addition & 1 deletion libdispatch/derror.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand Down
33 changes: 31 additions & 2 deletions libhdf5/hdf5filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
31 changes: 22 additions & 9 deletions libhdf5/hdf5open.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 */
Expand All @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions libhdf5/hdf5var.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++)
Expand Down Expand Up @@ -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++)
Expand Down
1 change: 1 addition & 0 deletions libnczarr/zinternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
51 changes: 36 additions & 15 deletions nc_test4/tst_filter.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash

if test "x$srcdir" = x ; then srcdir=`pwd`; fi
. ../test_common.sh
Expand Down Expand Up @@ -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}"
Expand Down Expand Up @@ -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
Expand Down