From 1834936ab0287312c7a752799fa7053fc7277043 Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Thu, 24 Mar 2022 12:03:55 -0500 Subject: Address comments from review. Add "version" field to H5FD_class_t struct and accompanying H5FD_CLASS_VERSION macro. --- src/H5FD.c | 11 ++++---- src/H5FDcore.c | 1 + src/H5FDdevelop.h | 4 +++ src/H5FDdirect.c | 1 + src/H5FDfamily.c | 1 + src/H5FDhdfs.c | 1 + src/H5FDint.c | 4 +-- src/H5FDlog.c | 1 + src/H5FDmirror.c | 1 + src/H5FDmpio.c | 31 +++++++++++++-------- src/H5FDmulti.c | 1 + src/H5FDros3.c | 1 + src/H5FDsec2.c | 1 + src/H5FDsplitter.c | 1 + src/H5FDstdio.c | 75 +++++++++++++++++++++++++------------------------- test/h5test.c | 1 + test/null_vfd_plugin.c | 1 + test/vfd.c | 1 + testpar/t_vfd.c | 2 -- 19 files changed, 83 insertions(+), 57 deletions(-) diff --git a/src/H5FD.c b/src/H5FD.c index 7d25eaf..1887e18 100644 --- a/src/H5FD.c +++ b/src/H5FD.c @@ -214,6 +214,8 @@ H5FDregister(const H5FD_class_t *cls) /* Check arguments */ if (!cls) HGOTO_ERROR(H5E_ARGS, H5E_UNINITIALIZED, H5I_INVALID_HID, "null class pointer is disallowed") + if (cls->version != H5FD_CLASS_VERSION) + HGOTO_ERROR(H5E_ARGS, H5E_VERSION, H5I_INVALID_HID, "wrong file driver version #") if (!cls->open || !cls->close) HGOTO_ERROR(H5E_ARGS, H5E_UNINITIALIZED, H5I_INVALID_HID, "'open' and/or 'close' methods are not defined") @@ -1553,7 +1555,6 @@ H5FDread_vector(H5FD_t *file, hid_t dxpl_id, uint32_t count, H5FD_mem_t types[], H5CX_set_dxpl(dxpl_id); /* Call private function */ - /* JRM -- review this */ /* (Note compensating for base addresses addition in internal routine) */ if (H5FD_read_vector(file, count, types, addrs, sizes, bufs) < 0) HGOTO_ERROR(H5E_VFL, H5E_READERROR, FAIL, "file vector read request failed") @@ -1631,7 +1632,7 @@ H5FDwrite_vector(H5FD_t *file, hid_t dxpl_id, uint32_t count, H5FD_mem_t types[] /* Set DXPL for operation */ H5CX_set_dxpl(dxpl_id); - /* Call private function */ /* JRM -- review this */ + /* Call private function */ /* (Note compensating for base address addition in internal routine) */ if (H5FD_write_vector(file, count, types, addrs, sizes, bufs) < 0) HGOTO_ERROR(H5E_VFL, H5E_WRITEERROR, FAIL, "file vector write request failed") @@ -1662,7 +1663,7 @@ done: * If the underlying VFD supports selection reads, pass the * call through directly. * - * If it doesn't, convert the vector write into a sequence + * If it doesn't, convert the selection read into a sequence * of individual reads. * * All reads are done according to the data transfer property @@ -1762,10 +1763,10 @@ done: * element_sizes[n] = element_sizes[i-1] for all n >= i and * < count. * - * If the underlying VFD supports selection reads, pass the + * If the underlying VFD supports selection writes, pass the * call through directly. * - * If it doesn't, convert the vector write into a sequence + * If it doesn't, convert the selection write into a sequence * of individual writes. * * All writes are done according to the data transfer property diff --git a/src/H5FDcore.c b/src/H5FDcore.c index b75fba3..0604316 100644 --- a/src/H5FDcore.c +++ b/src/H5FDcore.c @@ -152,6 +152,7 @@ static herr_t H5FD__core_delete(const char *filename, hid_t fapl_id); static inline const H5FD_core_fapl_t *H5FD__core_get_default_config(void); static const H5FD_class_t H5FD_core_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_CORE_VALUE, /* value */ "core", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDdevelop.h b/src/H5FDdevelop.h index 42df53e..e148101 100644 --- a/src/H5FDdevelop.h +++ b/src/H5FDdevelop.h @@ -25,6 +25,9 @@ /* Public Macros */ /*****************/ +/* H5FD_class_t struct version */ +#define H5FD_CLASS_VERSION 0x01 /* File driver struct version */ + /* Map "fractal heap" header blocks to 'ohdr' type file memory, since its * a fair amount of work to add a new kind of file memory and they are similar * enough to object headers and probably too minor to deserve their own type. @@ -160,6 +163,7 @@ typedef struct H5FD_t H5FD_t; /* Class information for each file driver */ typedef struct H5FD_class_t { + unsigned version; /**< File driver class struct version # */ H5FD_class_value_t value; const char * name; haddr_t maxaddr; diff --git a/src/H5FDdirect.c b/src/H5FDdirect.c index db71b76..25ee970 100644 --- a/src/H5FDdirect.c +++ b/src/H5FDdirect.c @@ -142,6 +142,7 @@ static herr_t H5FD__direct_unlock(H5FD_t *_file); static herr_t H5FD__direct_delete(const char *filename, hid_t fapl_id); static const H5FD_class_t H5FD_direct_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_DIRECT_VALUE, /* value */ "direct", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDfamily.c b/src/H5FDfamily.c index 8d2ddb4..66a1a68 100644 --- a/src/H5FDfamily.c +++ b/src/H5FDfamily.c @@ -112,6 +112,7 @@ static herr_t H5FD__family_delete(const char *filename, hid_t fapl_id); /* The class struct */ static const H5FD_class_t H5FD_family_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_FAMILY_VALUE, /* value */ "family", /* name */ HADDR_MAX, /* maxaddr */ diff --git a/src/H5FDhdfs.c b/src/H5FDhdfs.c index 6a10d43..f0ffb62 100644 --- a/src/H5FDhdfs.c +++ b/src/H5FDhdfs.c @@ -278,6 +278,7 @@ static herr_t H5FD__hdfs_truncate(H5FD_t *_file, hid_t dxpl_id, hbool_t closing static herr_t H5FD__hdfs_validate_config(const H5FD_hdfs_fapl_t *fa); static const H5FD_class_t H5FD_hdfs_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_HDFS_VALUE, /* value */ "hdfs", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDint.c b/src/H5FDint.c index 65e090a..6830f48 100644 --- a/src/H5FDint.c +++ b/src/H5FDint.c @@ -537,7 +537,7 @@ done: * * Note that it is not in general possible to convert a * vector write into a selection write, because each element - * in the vector read may have a different memory type. + * in the vector write may have a different memory type. * In contrast, selection writes are of a single type. * * Return: Success: SUCCEED @@ -1683,7 +1683,7 @@ done: * element_sizes[n] = element_sizes[i-1] for all n >= i and * < count. * - * If the underlying VFD supports selection reads, pass the + * If the underlying VFD supports selection writes, pass the * call through directly. * * If it doesn't, convert the vector write into a sequence diff --git a/src/H5FDlog.c b/src/H5FDlog.c index 376b4b0..4d2e705 100644 --- a/src/H5FDlog.c +++ b/src/H5FDlog.c @@ -180,6 +180,7 @@ static herr_t H5FD__log_unlock(H5FD_t *_file); static herr_t H5FD__log_delete(const char *filename, hid_t fapl_id); static const H5FD_class_t H5FD_log_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_LOG_VALUE, /* value */ "log", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDmirror.c b/src/H5FDmirror.c index def099b..0ab5345 100644 --- a/src/H5FDmirror.c +++ b/src/H5FDmirror.c @@ -160,6 +160,7 @@ static herr_t H5FD__mirror_unlock(H5FD_t *_file); static herr_t H5FD__mirror_verify_reply(H5FD_mirror_t *file); static const H5FD_class_t H5FD_mirror_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_MIRROR_VALUE, /* value */ "mirror", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDmpio.c b/src/H5FDmpio.c index 008ddf6..6b639f9 100644 --- a/src/H5FDmpio.c +++ b/src/H5FDmpio.c @@ -107,6 +107,7 @@ static herr_t H5FD__mpio_vector_build_types( /* The MPIO file driver information */ static const H5FD_class_t H5FD_mpio_g = { + H5FD_CLASS_VERSION, /* struct version */ H5_VFD_MPIO, /* value */ "mpio", /* name */ HADDR_MAX, /* maxaddr */ @@ -2289,12 +2290,16 @@ H5FD__mpio_read_vector(H5FD_t *_file, hid_t H5_ATTR_UNUSED dxpl_id, uint32_t cou /* The read is part of an independent operation. As a result, * we can't use MPI_File_set_view() (since it it a collective operation), - * and thus there is no point in setting up an MPI derived type, as - * (to the best of my knowledge) MPI I/O doesn't have support for - * non-contiguous I/O in independent mode. - * - * Thus we have to read in each element of the vector in a separate + * and thus we can't use the above code to construct the MPI datatypes. + * In the future, we could write code to detect when a contiguous slab + * in the file selection spans multiple vector elements and construct a + * memory datatype to match this larger block in the file, but for now + * just read in each element of the vector in a separate * MPI_File_read_at() call. + * + * We could also just detect the case when the entire file selection is + * contiguous, which would allow us to use + * H5FD__mpio_vector_build_types() to construct the memory datatype. */ #ifdef H5FDmpio_DEBUG @@ -2605,14 +2610,18 @@ H5FD__mpio_write_vector(H5FD_t *_file, hid_t H5_ATTR_UNUSED dxpl_id, uint32_t co hbool_t fixed_size = FALSE; size_t size; - /* The write is part of an independent operation. As a result, + /* The read is part of an independent operation. As a result, * we can't use MPI_File_set_view() (since it it a collective operation), - * and thus there is no point in setting up an MPI derived type, as - * (to the best of my knowledge) MPI I/O doesn't have support for - * non-contiguous I/O in independent mode. + * and thus we can't use the above code to construct the MPI datatypes. + * In the future, we could write code to detect when a contiguous slab + * in the file selection spans multiple vector elements and construct a + * memory datatype to match this larger block in the file, but for now + * just read in each element of the vector in a separate + * MPI_File_read_at() call. * - * Thus we have to write out each element of the vector in a separate - * MPI_File_write_at() call. + * We could also just detect the case when the entire file selection is + * contiguous, which would allow us to use + * H5FD__mpio_vector_build_types() to construct the memory datatype. */ #ifdef H5FDmpio_DEBUG diff --git a/src/H5FDmulti.c b/src/H5FDmulti.c index a273b95..20c538f 100644 --- a/src/H5FDmulti.c +++ b/src/H5FDmulti.c @@ -176,6 +176,7 @@ static herr_t H5FD_multi_ctl(H5FD_t *_file, uint64_t op_code, uint64_t flags, c /* The class struct */ static const H5FD_class_t H5FD_multi_g = { + H5FD_CLASS_VERSION, /* struct version */ H5_VFD_MULTI, /* value */ "multi", /* name */ HADDR_MAX, /* maxaddr */ diff --git a/src/H5FDros3.c b/src/H5FDros3.c index 0cf10a8..fcce76d 100644 --- a/src/H5FDros3.c +++ b/src/H5FDros3.c @@ -237,6 +237,7 @@ static herr_t H5FD__ros3_truncate(H5FD_t *_file, hid_t dxpl_id, hbool_t closing static herr_t H5FD__ros3_validate_config(const H5FD_ros3_fapl_t *fa); static const H5FD_class_t H5FD_ros3_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_ROS3_VALUE, /* value */ "ros3", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDsec2.c b/src/H5FDsec2.c index 8462eab..cc417070 100644 --- a/src/H5FDsec2.c +++ b/src/H5FDsec2.c @@ -143,6 +143,7 @@ static herr_t H5FD__sec2_ctl(H5FD_t *_file, uint64_t op_code, uint64_t flags, c void **output); static const H5FD_class_t H5FD_sec2_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_SEC2_VALUE, /* value */ "sec2", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDsplitter.c b/src/H5FDsplitter.c index 82752f6..124c54f 100644 --- a/src/H5FDsplitter.c +++ b/src/H5FDsplitter.c @@ -138,6 +138,7 @@ static herr_t H5FD__splitter_ctl(H5FD_t *_file, uint64_t op_code, uint64_t flag void **output); static const H5FD_class_t H5FD_splitter_g = { + H5FD_CLASS_VERSION, /* struct version */ H5FD_SPLITTER_VALUE, /* value */ "splitter", /* name */ MAXADDR, /* maxaddr */ diff --git a/src/H5FDstdio.c b/src/H5FDstdio.c index 22d6323..6624685 100644 --- a/src/H5FDstdio.c +++ b/src/H5FDstdio.c @@ -183,45 +183,46 @@ static herr_t H5FD_stdio_unlock(H5FD_t *_file); static herr_t H5FD_stdio_delete(const char *filename, hid_t fapl_id); static const H5FD_class_t H5FD_stdio_g = { - H5_VFD_STDIO, /* value */ - "stdio", /* name */ - MAXADDR, /* maxaddr */ - H5F_CLOSE_WEAK, /* fc_degree */ - H5FD_stdio_term, /* terminate */ - NULL, /* sb_size */ - NULL, /* sb_encode */ - NULL, /* sb_decode */ - 0, /* fapl_size */ - NULL, /* fapl_get */ - NULL, /* fapl_copy */ - NULL, /* fapl_free */ - 0, /* dxpl_size */ - NULL, /* dxpl_copy */ - NULL, /* dxpl_free */ - H5FD_stdio_open, /* open */ - H5FD_stdio_close, /* close */ - H5FD_stdio_cmp, /* cmp */ - H5FD_stdio_query, /* query */ - NULL, /* get_type_map */ - H5FD_stdio_alloc, /* alloc */ - NULL, /* free */ - H5FD_stdio_get_eoa, /* get_eoa */ - H5FD_stdio_set_eoa, /* set_eoa */ - H5FD_stdio_get_eof, /* get_eof */ - H5FD_stdio_get_handle, /* get_handle */ - H5FD_stdio_read, /* read */ - H5FD_stdio_write, /* write */ - NULL, /* read_vector */ - NULL, /* write_vector */ + H5FD_CLASS_VERSION, /* struct version */ + H5_VFD_STDIO, /* value */ + "stdio", /* name */ + MAXADDR, /* maxaddr */ + H5F_CLOSE_WEAK, /* fc_degree */ + H5FD_stdio_term, /* terminate */ + NULL, /* sb_size */ + NULL, /* sb_encode */ + NULL, /* sb_decode */ + 0, /* fapl_size */ + NULL, /* fapl_get */ + NULL, /* fapl_copy */ + NULL, /* fapl_free */ + 0, /* dxpl_size */ + NULL, /* dxpl_copy */ + NULL, /* dxpl_free */ + H5FD_stdio_open, /* open */ + H5FD_stdio_close, /* close */ + H5FD_stdio_cmp, /* cmp */ + H5FD_stdio_query, /* query */ + NULL, /* get_type_map */ + H5FD_stdio_alloc, /* alloc */ + NULL, /* free */ + H5FD_stdio_get_eoa, /* get_eoa */ + H5FD_stdio_set_eoa, /* set_eoa */ + H5FD_stdio_get_eof, /* get_eof */ + H5FD_stdio_get_handle, /* get_handle */ + H5FD_stdio_read, /* read */ + H5FD_stdio_write, /* write */ + NULL, /* read_vector */ + NULL, /* write_vector */ NULL, /* read_selection */ NULL, /* write_selection */ - H5FD_stdio_flush, /* flush */ - H5FD_stdio_truncate, /* truncate */ - H5FD_stdio_lock, /* lock */ - H5FD_stdio_unlock, /* unlock */ - H5FD_stdio_delete, /* del */ - NULL, /* ctl */ - H5FD_FLMAP_DICHOTOMY /* fl_map */ + H5FD_stdio_flush, /* flush */ + H5FD_stdio_truncate, /* truncate */ + H5FD_stdio_lock, /* lock */ + H5FD_stdio_unlock, /* unlock */ + H5FD_stdio_delete, /* del */ + NULL, /* ctl */ + H5FD_FLMAP_DICHOTOMY /* fl_map */ }; /*------------------------------------------------------------------------- diff --git a/test/h5test.c b/test/h5test.c index 1403649..ac15043 100644 --- a/test/h5test.c +++ b/test/h5test.c @@ -1773,6 +1773,7 @@ dummy_vfd_write(H5FD_t H5_ATTR_UNUSED *_file, H5FD_mem_t H5_ATTR_UNUSED type, hi /* Dummy VFD with the minimum parameters to make a VFD that can be registered */ #define DUMMY_VFD_VALUE (H5FD_class_value_t)155 static const H5FD_class_t H5FD_dummy_g = { + H5FD_CLASS_VERSION, /* struct version */ DUMMY_VFD_VALUE, /* value */ "dummy", /* name */ 1, /* maxaddr */ diff --git a/test/null_vfd_plugin.c b/test/null_vfd_plugin.c index 125b510..ca59939 100644 --- a/test/null_vfd_plugin.c +++ b/test/null_vfd_plugin.c @@ -35,6 +35,7 @@ static herr_t H5FD_null_set_eoa(H5FD_t *_file, H5FD_mem_t type, haddr_t addr); static haddr_t H5FD_null_get_eof(const H5FD_t *_file, H5FD_mem_t type); static const H5FD_class_t H5FD_null_g = { + H5FD_CLASS_VERSION, /* struct version */ NULL_VFD_VALUE, /* value */ NULL_VFD_NAME, /* name */ 1, /* maxaddr */ diff --git a/test/vfd.c b/test/vfd.c index 78efb6f..d1c95ce 100644 --- a/test/vfd.c +++ b/test/vfd.c @@ -3600,6 +3600,7 @@ H5FD__ctl_test_vfd_ctl(H5FD_t H5_ATTR_UNUSED *_file, uint64_t op_code, uint64_t /* Minimal VFD for ctl feature tests */ static const H5FD_class_t H5FD_ctl_test_vfd_g = { + H5FD_CLASS_VERSION, /* struct version */ (H5FD_class_value_t)201, /* value */ "ctl_test_vfd", /* name */ HADDR_MAX, /* maxaddr */ diff --git a/testpar/t_vfd.c b/testpar/t_vfd.c index 09a7103..2072afe 100644 --- a/testpar/t_vfd.c +++ b/testpar/t_vfd.c @@ -1,6 +1,5 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * Copyright by The HDF Group. * - * Copyright by the Board of Trustees of the University of Illinois. * * All rights reserved. * * * * This file is part of HDF5. The full HDF5 copyright notice, including * @@ -12,7 +11,6 @@ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ /* Programmer: John Mainzer - * 7/13/15 * * This file is a catchall for parallel VFD tests. */ -- cgit v0.12