Add tests for the two expected failure modes for variable-length (VL) strings

in VFD SWMR mode.

6 files changed, 243 insertions, 115 deletions

diff --git a/test/Makefile.am b/test/Makefile.am
index ad63093..953816c 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -46,7 +46,7 @@ SCRIPT_DEPEND = error_test$(EXEEXT) err_compat$(EXEEXT) links_env$(EXEEXT) \
     vfd_swmr_generator$(EXEEXT) vfd_swmr_reader$(EXEEXT) vfd_swmr_writer$(EXEEXT)  \
     vfd_swmr_remove_reader$(EXEEXT) vfd_swmr_remove_writer$(EXEEXT)  \
     vfd_swmr_addrem_writer$(EXEEXT) vfd_swmr_sparse_reader$(EXEEXT) \
-    vfd_swmr_sparse_writer$(EXEEXT) vfd_swmr_vlstr$(EXEEXT) \
+    vfd_swmr_sparse_writer$(EXEEXT) vfd_swmr_vlstr_writer$(EXEEXT) \
     vfd_swmr_vlstr_reader$(EXEEXT) \
     vds_env$(EXEEXT) \
     vds_swmr_gen$(EXEEXT) vds_swmr_reader$(EXEEXT) vds_swmr_writer$(EXEEXT)
@@ -97,7 +97,7 @@ check_PROGRAMS=$(TEST_PROG) error_test err_compat tcheck_version \
     vfd_swmr_generator vfd_swmr_reader vfd_swmr_writer \
     vfd_swmr_remove_reader vfd_swmr_remove_writer vfd_swmr_addrem_writer \
     vfd_swmr_sparse_reader vfd_swmr_sparse_writer \
-    vfd_swmr_vlstr vfd_swmr_vlstr_reader \
+    vfd_swmr_vlstr_reader vfd_swmr_vlstr_writer \
     swmr_check_compat_vfd vds_env vds_swmr_gen vds_swmr_reader vds_swmr_writer
 if HAVE_SHARED_CONDITIONAL
   check_PROGRAMS+= filter_plugin vol_plugin
diff --git a/test/testvfdswmr.sh.in b/test/testvfdswmr.sh.in
index 8d81b59..626b9ad 100644
--- a/test/testvfdswmr.sh.in
+++ b/test/testvfdswmr.sh.in
@@ -116,7 +116,7 @@ if [ $rc -ne 0 ] ; then
     exit 0
 fi
 
-all_tests="generator expand shrink expand_shrink sparse"
+all_tests="generator expand shrink expand_shrink sparse vlstr_null vlstr_oob"
 tests=${all_tests}
 
 if [ $# -gt 0 ]; then
@@ -444,7 +444,7 @@ do
 
 	if [ ${do_sparse:-no} = yes ]; then
 		echo
-		echo "## Sparse writer test - test writing to random locations in the dataset"
+		echo "## Sparse writer test - write random dataset locations"
 
 		# Launch the Generator
 		# NOTE: Random seed is shared between readers and writers and is
@@ -527,6 +527,61 @@ do
     done
 done
 
+for ty in null oob; do
+
+	if [ ${ty} = null ]; then
+		[ ${do_vlstr_null:-no} = no ] && continue
+		echo
+		echo "## VL string 1 - expect to read NULL"
+	else
+		[ ${do_vlstr_oob:-no} = no ] && continue
+		echo
+		echo "## VL string 2 - expect out-of-bounds access"
+	fi
+
+	echo launch vfd_swmr_vlstr_writer
+	catch_out_err_and_rc vfd_swmr_vlstr_writer \
+	    ../vfd_swmr_vlstr_writer -q -t ${ty} &
+	pid_writer=$!
+
+	# pause?
+
+	catch_out_err_and_rc vfd_swmr_vlstr_reader \
+	    ../vfd_swmr_vlstr_reader -q -t ${ty} &
+	pid_reader=$!
+
+	# Wait for the reader to finish before signalling the
+	# writer to quit: the writer holds the file open so that the
+	# reader will find the shadow file when it opens
+	# the .h5 file.
+	wait $pid_reader
+	kill -USR1 $(cat vfd_swmr_vlstr_writer.pid)
+	wait $pid_writer
+
+	# Collect exit code of the reader
+	if [ $(cat vfd_swmr_vlstr_reader.rc) -ne 0 ]; then
+		echo reader had error
+		nerrors=$((nerrors + 1))
+	fi
+
+	# Collect exit code of the writer
+	if [ $(cat vfd_swmr_vlstr_writer.rc) -ne 0 ]; then
+	    echo writer had error
+	    nerrors=$((nerrors + 1))
+	fi
+
+	# Check for error and exit if one occured
+	if test $nerrors -ne 0 ; then
+	    echo "VFD SWMR tests failed with $nerrors errors."
+	    echo "(Writer and reader output preserved)"
+	    exit 1
+	fi
+
+	# Clean up output files
+	rm -f vfd_swmr_vlstr_writer.{out,rc}
+	rm -f vfd_swmr_vlstr_reader.*.{out,rc}
+done
+
 ###############################################################################
 ## Report and exit
 ###############################################################################
diff --git a/test/vfd_swmr_common.c b/test/vfd_swmr_common.c
index f6156fe..2bf4619 100644
--- a/test/vfd_swmr_common.c
+++ b/test/vfd_swmr_common.c
@@ -29,6 +29,23 @@
 #include "h5test.h"
 #include "vfd_swmr_common.h"
 
+static const hid_t badhid = H5I_INVALID_HID;
+
+int verbosity = 2;
+
+void
+dbgf(int level, const char *fmt, ...)
+{
+    va_list ap;
+
+    if (verbosity < level)
+        return;
+
+    va_start(ap, fmt);
+    (void)vprintf(fmt, ap);
+    va_end(ap);
+}
+
 estack_state_t
 disable_estack(void)
 {
@@ -83,8 +100,8 @@ await_signal(hid_t fid)
     for (;;) {
         const int rc = sigtimedwait(&sleepset, NULL, &tick);
 
-        if (rc == SIGUSR1) {
-            printf("Received SIGUSR1, wrapping things up.\n");
+        if (rc == SIGUSR1 || rc == SIGINT) {
+            printf("Received %s, wrapping things up.\n", (rc == SIGUSR1) ? "SIGUSR1" : "SIGINT");
             break;
         } else if (rc == -1 && errno == EAGAIN) {
             estack_state_t es;
@@ -104,3 +121,48 @@ await_signal(hid_t fid)
             err(EXIT_FAILURE, "%s: sigtimedwait", __func__);
     }
 }
+
+hid_t
+vfd_swmr_create_fapl(bool writer, bool only_meta_pages, bool use_vfd_swmr)
+{
+    H5F_vfd_swmr_config_t config;
+    hid_t fapl;
+
+    /* Create file access property list */
+    if((fapl = h5_fileaccess()) < 0) {
+        warnx("h5_fileaccess");
+        return badhid;
+    }
+
+    /* FOR NOW: set to use latest format, the "old" parameter is not used */
+    if(H5Pset_libver_bounds(fapl, H5F_LIBVER_LATEST, H5F_LIBVER_LATEST) < 0) {
+        warnx("H5Pset_libver_bounds");
+        return badhid;
+    }
+
+    /*
+     * Set up to open the file with VFD SWMR configured.
+     */
+
+    /* Enable page buffering */
+    if(H5Pset_page_buffer_size(fapl, 4096, only_meta_pages ? 100 : 0, 0) < 0) {
+        warnx("H5Pset_page_buffer_size");
+        return badhid;
+    }
+
+    memset(&config, 0, sizeof(config));
+
+    config.version = H5F__CURR_VFD_SWMR_CONFIG_VERSION;
+    config.tick_len = 1;
+    config.max_lag = 5;
+    config.writer = writer;
+    config.md_pages_reserved = 128;
+    HDstrcpy(config.md_file_path, "./my_md_file");
+
+    /* Enable VFD SWMR configuration */
+    if(use_vfd_swmr && H5Pset_vfd_swmr_config(fapl, &config) < 0) {
+        warnx("H5Pset_vfd_swmr_config");
+        return badhid;
+    }
+    return fapl;
+}
diff --git a/test/vfd_swmr_common.h b/test/vfd_swmr_common.h
index ac08503..7998d29 100644
--- a/test/vfd_swmr_common.h
+++ b/test/vfd_swmr_common.h
@@ -61,6 +61,12 @@ typedef struct {
     uint8_t info[DTYPE_SIZE];   /* "Other" information for this record */
 } symbol_t;
 
+typedef enum _testsel {
+  TEST_NONE = 0
+, TEST_NULL
+, TEST_OOB
+} testsel_t;
+
 /********************/
 /* Global Variables */
 /********************/
@@ -87,9 +93,14 @@ H5TEST_DLL int print_metadata_retries_info(hid_t fid);
 H5TEST_DLL void block_signals(sigset_t *);
 H5TEST_DLL void restore_signals(sigset_t *);
 H5TEST_DLL void await_signal(hid_t);
+H5TEST_DLL hid_t vfd_swmr_create_fapl(bool, bool, bool);
+
+H5TEST_DLL void dbgf(int, const char *, ...) H5_ATTR_FORMAT(printf, 2, 3);
 
 #ifdef __cplusplus
 }
 #endif
 
+extern int verbosity;
+
 #endif /* _SWMR_COMMON_H */
diff --git a/test/vfd_swmr_vlstr_reader.c b/test/vfd_swmr_vlstr_reader.c
index dcd4468..e797731 100644
--- a/test/vfd_swmr_vlstr_reader.c
+++ b/test/vfd_swmr_vlstr_reader.c
@@ -29,7 +29,7 @@
 #include "testhdf5.h"
 #include "vfd_swmr_common.h"
 
-enum _step {
+typedef enum _step {
   CREATE = 0
 , LENGTHEN
 , SHORTEN
@@ -39,33 +39,33 @@ enum _step {
 
 static const hid_t badhid = H5I_INVALID_HID; // abbreviate
 static bool caught_out_of_bounds = false;
+static bool read_null = false;
 
-static void
-read_vl_dset(hid_t dset, hid_t type, hid_t space, char *data)
-{
-    if (H5Dread(dset, type, H5S_ALL, H5S_ALL, H5P_DEFAULT, &data) < 0)
-        errx(EXIT_FAILURE, "%s: H5Dwrite", __func__);
-}
-
-static hid_t
-open_vl_dset(hid_t file, hid_t type, const char *name)
+static bool
+read_vl_dset(hid_t dset, hid_t type, char **data)
 {
-    hid_t dset;
-
-    dset = H5Dopen(file, name, H5P_DEFAULT);
+    bool success;
+    estack_state_t es;
 
-    if (dset == badhid)
-        errx(EXIT_FAILURE, "H5Dopen");
+    es = disable_estack();
+    success = H5Dread(dset, type, H5S_ALL, H5S_ALL, H5P_DEFAULT, data) < 0;
+    if (*data == NULL) {
+        read_null = true;
+        return false;
+    }
+    restore_estack(es);
 
-    return dset;
+    return success;
 }
 
 static void
 usage(const char *progname)
 {
-    fprintf(stderr, "usage: %s [-W] [-V]\n", progname);
-    fprintf(stderr, "\n  -W: do not wait for SIGUSR1\n");
+    fprintf(stderr, "usage: %s [-W] [-V] [-t (oob|null)] \n", progname);
+    fprintf(stderr, "\n  -S: do not use VFD SWMR\n");
     fprintf(stderr,   "  -n: number of test steps to perform\n");
+    fprintf(stderr,   "  -q: be quiet: few/no progress messages\n");
+    fprintf(stderr,   "  -t (oob|null): select out-of-bounds or NULL test\n");
     exit(EXIT_FAILURE);
 }
 
@@ -74,9 +74,9 @@ H5HG_trap(const char *reason)
 {
     if (strcmp(reason, "out of bounds") == 0) {
         caught_out_of_bounds = true;
-        return false;
+        return true;
     }
-    return true;
+    return false;
 }
 
 int
@@ -84,25 +84,23 @@ main(int argc, char **argv)
 {
     hid_t fapl, fid, space, type;
     hid_t dset[2];
-    char content[2][96];
+    char *content[2];
     char name[2][96];
-    H5F_vfd_swmr_config_t config;
-    sigset_t oldsigs;
-    herr_t ret;
-    bool wait_for_signal = true;
-    const hsize_t dims = 1;
     int ch, i, ntimes = 100;
     unsigned long tmp;
+    bool use_vfd_swmr = true;
     char *end;
+    const long millisec_in_nanosecs = 1000 * 1000;
     const struct timespec delay =
-        {.tv_sec = 0, .tv_nsec = 1000 * 1000 * 1000 / 10};
+        {.tv_sec = 0, .tv_nsec = millisec_in_nanosecs * 11 / 10};
+    testsel_t sel = TEST_NONE;
 
     assert(H5T_C_S1 != badhid);
 
-    while ((ch = getopt(argc, argv, "Wn:")) != -1) {
+    while ((ch = getopt(argc, argv, "Sn:qt:")) != -1) {
         switch(ch) {
-        case 'W':
-            wait_for_signal = false;
+        case 'S':
+            use_vfd_swmr = false;
             break;
         case 'n':
             errno = 0;
@@ -115,6 +113,17 @@ main(int argc, char **argv)
                 errx(EXIT_FAILURE, "`-n` argument `%lu` too large", tmp);
             ntimes = (int)tmp;
             break;
+        case 'q':
+            verbosity = 1;
+            break;
+        case 't':
+            if (strcmp(optarg, "oob") == 0)
+                sel = TEST_OOB;
+            else if (strcmp(optarg, "null") == 0)
+                sel = TEST_NULL;
+            else
+                usage(argv[0]);
+            break;
         default:
             usage(argv[0]);
             break;
@@ -127,33 +136,8 @@ main(int argc, char **argv)
         errx(EXIT_FAILURE, "unexpected command-line arguments");
 
     /* Create file access property list */
-    if((fapl = h5_fileaccess()) < 0)
-        errx(EXIT_FAILURE, "h5_fileaccess");
-
-    /* FOR NOW: set to use latest format, the "old" parameter is not used */
-    if(H5Pset_libver_bounds(fapl, H5F_LIBVER_LATEST, H5F_LIBVER_LATEST) < 0)
-        errx(EXIT_FAILURE, "H5Pset_libver_bounds");
-
-    /*
-     * Set up to open the file with VFD SWMR configured.
-     */
-
-    /* Enable page buffering */
-    if(H5Pset_page_buffer_size(fapl, 4096, 100, 0) < 0)
-        errx(EXIT_FAILURE, "H5Pset_page_buffer_size");
-
-    memset(&config, 0, sizeof(config));
-
-    config.version = H5F__CURR_VFD_SWMR_CONFIG_VERSION;
-    config.tick_len = 1;
-    config.max_lag = 5;
-    config.writer = true;
-    config.md_pages_reserved = 128;
-    HDstrcpy(config.md_file_path, "./my_md_file");
-
-    /* Enable VFD SWMR configuration */
-    if(H5Pset_vfd_swmr_config(fapl, &config) < 0)
-        errx(EXIT_FAILURE, "H5Pset_vfd_swmr_config");
+    if ((fapl = vfd_swmr_create_fapl(false, sel == TEST_OOB, use_vfd_swmr)) < 0)
+        errx(EXIT_FAILURE, "vfd_swmr_create_fapl");
 
     fid = H5Fopen("vfd_swmr_vlstr.h5", H5F_ACC_RDONLY, fapl);
 
@@ -177,35 +161,54 @@ main(int argc, char **argv)
     if (fid == badhid)
         errx(EXIT_FAILURE, "H5Fcreate");
 
-    block_signals(&oldsigs);
-
     /* content 1 seq 1 short
      * content 1 seq 1 long long long long long long long long
      * content 1 seq 1 medium medium medium
      */
-    for (i = 0; i < ntimes; i++) {
+    for (i = 0;
+         !caught_out_of_bounds && i < ntimes;
+         (i % 2 == 0) ? nanosleep(&delay, NULL) : 0, i++) {
+        estack_state_t es;
         const int ndsets = 2;
         const int which = i % ndsets;
-        fprintf(stderr, "iteration %d which %d\n", i, which);
+        int nconverted;
+        struct {
+            int which;
+            int seq;
+            char tail[96];
+        } scanned_content;
+
+        dbgf(2, "iteration %d which %d", i, which);
         (void)snprintf(name[which], sizeof(name[which]),
             "dset-%d", which);
-        dset[which] = open_vl_dset(fid, type, name[which]);
-        read_vl_dset(dset[which], type, space, content[which]);
-#if 0
-        (void)snprintf(content[which], sizeof(content[which]),
-            "content %d seq %d %s", which, seq, tail);
-#endif
-        if (caught_out_of_bounds) {
-            fprintf(stderr, "caught out of bounds\n");
-            break;
+        es = disable_estack();
+        dset[which] = H5Dopen(fid, name[which], H5P_DEFAULT);
+        restore_estack(es);
+        if (caught_out_of_bounds || dset[which] == badhid) {
+            dbgf(2, ": couldn't open\n");
+            continue;
+        }
+        if (!read_vl_dset(dset[which], type, &content[which])) {
+            H5Dclose(dset[which]);
+            dbgf(2, ": couldn't read\n");
+            continue;
         }
-        nanosleep(&delay, NULL);
+        nconverted = sscanf(content[which], "content %d seq %d %96s",
+            &scanned_content.which, &scanned_content.seq, scanned_content.tail);
+        if (nconverted != 3) {
+            dbgf(2, ": couldn't scan\n");
+            continue;
+        }
+        dbgf(2, ": read which %d seq %d tail %s\n",
+            scanned_content.which, scanned_content.seq, scanned_content.tail);
+        H5Dclose(dset[which]);
     }
 
-    if (wait_for_signal)
-        await_signal(fid);
+    if (caught_out_of_bounds)
+        fprintf(stderr, "caught out of bounds\n");
 
-    restore_signals(&oldsigs);
+    if (read_null)
+        fprintf(stderr, "read NULL\n");
 
     if (H5Pclose(fapl) < 0)
         errx(EXIT_FAILURE, "H5Pclose(fapl)");
@@ -219,5 +222,10 @@ main(int argc, char **argv)
     if (H5Fclose(fid) < 0)
         errx(EXIT_FAILURE, "H5Fclose");
 
+    if (sel == TEST_OOB)
+        return caught_out_of_bounds ? EXIT_SUCCESS : EXIT_FAILURE;
+    else if (sel == TEST_NULL)
+        return read_null ? EXIT_SUCCESS : EXIT_FAILURE;
+
     return EXIT_SUCCESS;
 }
diff --git a/test/vfd_swmr_vlstr.c b/test/vfd_swmr_vlstr_writer.c
index 3c73fd2..80a2543 100644
--- a/test/vfd_swmr_vlstr.c
+++ b/test/vfd_swmr_vlstr_writer.c
@@ -101,20 +101,23 @@ print_cache_hits(H5C_t *cache)
     int i;
 
     for (i = 0; i < H5AC_NTYPES; i++) {
-        printf("type-%d cache hits %" PRId64 "%s\n",
+        dbgf(3, "type-%d cache hits %" PRId64 "%s\n",
             i, cache->hits[i], (i == H5AC_GHEAP_ID) ? " *" : "");
     }
-    printf("\n");
+    dbgf(3, "\n");
 }
 
 static void
 usage(const char *progname)
 {
     fprintf(stderr, "usage: %s [-W] [-V]\n", progname);
-    fprintf(stderr, "\n  -W: do not wait for SIGUSR1\n");
+    fprintf(stderr, "\n  -W: do not wait for SIGINT or SIGUSR1\n");
+    fprintf(stderr, "\n  -S: do not use VFD SWMR\n");
     fprintf(stderr,   "  -f: use fixed-length string\n");
     fprintf(stderr,   "      (default: variable-length string)\n");
     fprintf(stderr,   "  -n: number of test steps to perform\n");
+    fprintf(stderr,   "  -q: be quiet: few/no progress messages\n");
+    fprintf(stderr,   "  -t (oob|null): select out-of-bounds or NULL test\n");
     exit(EXIT_FAILURE);
 }
 
@@ -137,7 +140,6 @@ main(int argc, char **argv)
     char name[2][96];
     H5F_t *f;
     H5C_t *cache;
-    H5F_vfd_swmr_config_t config;
     sigset_t oldsigs;
     herr_t ret;
     bool variable = true, wait_for_signal = true;
@@ -145,13 +147,18 @@ main(int argc, char **argv)
     int ch, i, ntimes = 100;
     unsigned long tmp;
     char *end;
+    bool use_vfd_swmr = true;
     const struct timespec delay =
         {.tv_sec = 0, .tv_nsec = 1000 * 1000 * 1000 / 10};
+    testsel_t sel = TEST_NONE;
 
     assert(H5T_C_S1 != badhid);
 
-    while ((ch = getopt(argc, argv, "Wfn:")) != -1) {
+    while ((ch = getopt(argc, argv, "SWfn:qt:")) != -1) {
         switch(ch) {
+        case 'S':
+            use_vfd_swmr = false;
+            break;
         case 'W':
             wait_for_signal = false;
             break;
@@ -169,6 +176,17 @@ main(int argc, char **argv)
                 errx(EXIT_FAILURE, "`-n` argument `%lu` too large", tmp);
             ntimes = (int)tmp;
             break;
+        case 'q':
+            verbosity = 1;
+            break;
+        case 't':
+            if (strcmp(optarg, "oob") == 0)
+                sel = TEST_OOB;
+            else if (strcmp(optarg, "null") == 0)
+                sel = TEST_NULL;
+            else
+                usage(argv[0]);
+            break;
         default:
             usage(argv[0]);
             break;
@@ -180,34 +198,8 @@ main(int argc, char **argv)
     if (argc > 0)
         errx(EXIT_FAILURE, "unexpected command-line arguments");
 
-    /* Create file access property list */
-    if((fapl = h5_fileaccess()) < 0)
-        errx(EXIT_FAILURE, "h5_fileaccess");
-
-    /* FOR NOW: set to use latest format, the "old" parameter is not used */
-    if(H5Pset_libver_bounds(fapl, H5F_LIBVER_LATEST, H5F_LIBVER_LATEST) < 0)
-        errx(EXIT_FAILURE, "H5Pset_libver_bounds");
-
-    /*
-     * Set up to open the file with VFD SWMR configured.
-     */
-
-    /* Enable page buffering */
-    if(H5Pset_page_buffer_size(fapl, 4096, 100, 0) < 0)
-        errx(EXIT_FAILURE, "H5Pset_page_buffer_size");
-
-    memset(&config, 0, sizeof(config));
-
-    config.version = H5F__CURR_VFD_SWMR_CONFIG_VERSION;
-    config.tick_len = 1;
-    config.max_lag = 5;
-    config.writer = true;
-    config.md_pages_reserved = 128;
-    HDstrcpy(config.md_file_path, "./my_md_file");
-
-    /* Enable VFD SWMR configuration */
-    if(H5Pset_vfd_swmr_config(fapl, &config) < 0)
-        errx(EXIT_FAILURE, "H5Pset_vfd_swmr_config");
+    if ((fapl = vfd_swmr_create_fapl(true, sel == TEST_OOB, use_vfd_swmr)) < 0)
+        errx(EXIT_FAILURE, "vfd_swmr_create_fapl");
 
     if ((fcpl = H5Pcreate(H5P_FILE_CREATE)) < 0)
         errx(EXIT_FAILURE, "H5Pcreate");
@@ -262,7 +254,7 @@ main(int argc, char **argv)
         const int step = i % NSTEPS;
         const int which = (i / NSTEPS) % ndsets;
         const int seq = i / (ndsets * NSTEPS);
-        fprintf(stderr, "iteration %d which %d step %d seq %d\n",
+        dbgf(2, "iteration %d which %d step %d seq %d\n",
             i, which, step, seq);
         switch (step) {
         case CREATE:
@@ -304,7 +296,7 @@ main(int argc, char **argv)
         nanosleep(&delay, NULL);
     }
 
-    if (wait_for_signal)
+    if (use_vfd_swmr && wait_for_signal)
         await_signal(fid);
 
     restore_signals(&oldsigs);