diff --git a/release_docs/CHANGELOG.md b/release_docs/CHANGELOG.md index c9d5ac47ad6..49d4d6c4501 100644 --- a/release_docs/CHANGELOG.md +++ b/release_docs/CHANGELOG.md @@ -190,6 +190,15 @@ The `h5repack` tool now obtains its default low and high library version bounds ## Tools +### Fixed h5repack silently dropping a declared cd_nelmts for user-defined filters + + `h5repack -f UD=,,` with no values following `cd_nelmts` silently + treated the declared count as 0 instead of validating it, because the trailing token (with + no comma after it) was never committed to `cd_nelmts` inside the parser. `parse_filter()` now + commits the trailing token to whichever UD field is still pending, so a declared, unfulfilled + `cd_nelmts` is correctly rejected with "incorrect number of compression parameters" instead of + being silently coerced to 0. + ## Performance ## Fortran API diff --git a/tools/src/h5repack/h5repack_parse.c b/tools/src/h5repack/h5repack_parse.c index dc2f9aed7f3..b12034a7f50 100644 --- a/tools/src/h5repack/h5repack_parse.c +++ b/tools/src/h5repack/h5repack_parse.c @@ -58,7 +58,7 @@ parse_filter(const char *str, unsigned *n_objs, filter_info_t *filt, pack_opt_t size_t i, m, u; char c; size_t len = strlen(str); - int f, k, l, p, q, end_obj = -1, no_param = 0; + int f = -1, k, l = -1, p = -1, q, end_obj = -1, no_param = 0; unsigned j, n; char sobj[MAX_NC_NAME]; char scomp[16] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; @@ -297,8 +297,19 @@ parse_filter(const char *str, unsigned *n_objs, filter_info_t *filt, pack_opt_t } /*if */ done_filter_params:; - if ((strcmp(scomp, "UD") == 0) && (filt->cd_nelmts == 0)) - j = 0; + if (strcmp(scomp, "UD") == 0) { + /* the trailing token (no comma after it) was never committed inside + * the loop above; commit it to whichever UD field is still pending, + * mirroring the comma-triggered branch exactly */ + if (l == -1) + filt->filtn = atoi(stype); + else if (f == -1) + filt->filt_flag = (unsigned)strtoul(stype, NULL, 0); + else if (p == -1) + filt->cd_nelmts = strtoull(stype, NULL, 0); + else if (filt->cd_nelmts > 0) + filt->cd_values[j++] = (unsigned)strtoul(stype, NULL, 0); + } else filt->cd_values[j++] = (unsigned)strtoul(stype, NULL, 0); } diff --git a/tools/test/h5repack/CMakeTests.cmake b/tools/test/h5repack/CMakeTests.cmake index e22562f839b..8361eb9d305 100644 --- a/tools/test/h5repack/CMakeTests.cmake +++ b/tools/test/h5repack/CMakeTests.cmake @@ -1892,6 +1892,9 @@ if (BUILD_SHARED_LIBS) ADD_H5_UD_TEST (plugin_test_ex 1 h5repack_layout.h5 --enable-error-stack -v -f UD=257,0,1,9,9,9) # check for extra parameters, which are ignored when nelms is 0 ADD_H5_UD_TEST (plugin_zero_extra 0 h5repack_layout.h5 --enable-error-stack -v -f UD=250,0,0,1,2) + # check for a declared cd_nelmts with no values following it (filter 250 does no + # internal cd_values validation, so this only fails if parse_filter() catches it itself) + ADD_H5_UD_TEST (plugin_test_nelmts_no_values 1 h5repack_layout.h5 --enable-error-stack -v -f UD=250,0,1) endif () ##############################################################################