From 8e4e9e5f3fc6da29afe1588f8b31df3561986f99 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Tue, 5 Sep 2023 14:43:33 +0200 Subject: [PATCH 1/2] Change type of `offset` arg in `H5Pset_external` to `HDoff_t` The `off_t` type is only 32-bit on Windows, which makes it impossible to link to higher offsets in large files. The `H5O_efl_entry_t` struct defines its `offset` field already as `HDoff_t`, so no additional conversion is needed. --- bin/trace | 1 + src/H5Pdcpl.c | 4 ++-- src/H5Ppublic.h | 17 ++++++++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/bin/trace b/bin/trace index c620b805368..5ea9b96392a 100755 --- a/bin/trace +++ b/bin/trace @@ -109,6 +109,7 @@ $Source = ""; "H5M_iterate_t" => 'MI', "H5FD_mem_t" => "Mt", "off_t" => "o", + "HDoff_t" => "Ho", "H5O_iterate1_t" => "Oi", "H5O_iterate2_t" => "OI", "H5O_mcdt_search_cb_t" => "Os", diff --git a/src/H5Pdcpl.c b/src/H5Pdcpl.c index 5254f6db0d2..8952bcfb89f 100644 --- a/src/H5Pdcpl.c +++ b/src/H5Pdcpl.c @@ -2599,7 +2599,7 @@ H5Pget_chunk_opts(hid_t plist_id, unsigned *options /*out*/) *------------------------------------------------------------------------- */ herr_t -H5Pset_external(hid_t plist_id, const char *name, off_t offset, hsize_t size) +H5Pset_external(hid_t plist_id, const char *name, HDoff_t offset, hsize_t size) { size_t idx; hsize_t total, tmp; @@ -2608,7 +2608,7 @@ H5Pset_external(hid_t plist_id, const char *name, off_t offset, hsize_t size) herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_API(FAIL) - H5TRACE4("e", "i*soh", plist_id, name, offset, size); + H5TRACE4("e", "i*sHoh", plist_id, name, offset, size); /* Check arguments */ if (!name || !*name) diff --git a/src/H5Ppublic.h b/src/H5Ppublic.h index 124f6614c74..72d805e9a38 100644 --- a/src/H5Ppublic.h +++ b/src/H5Ppublic.h @@ -5570,6 +5570,21 @@ H5_DLL herr_t H5Pget_mpi_params(hid_t fapl_id, MPI_Comm *comm, MPI_Info *info); */ H5_DLL herr_t H5Pset_mpi_params(hid_t fapl_id, MPI_Comm comm, MPI_Info info); #endif /* H5_HAVE_PARALLEL */ + +#ifdef H5_HAVE_WIN32_API +/* off_t exists on Windows, but is always a 32-bit long, even on 64-bit Windows, + * so we define HDoff_t to be __int64, which is the type of the st_size field + * of the _stati64 struct. + */ +#define HDoff_t __int64 +#endif /* H5_HAVE_WIN32_API */ +#ifndef H5_HAVE_WIN32_API +/* These definitions differ in Windows and are defined in + * H5win32defs for that platform. + */ +#define HDoff_t off_t +#endif + /** * \ingroup FAPL * @@ -6353,7 +6368,7 @@ H5_DLL herr_t H5Pset_dset_no_attrs_hint(hid_t dcpl_id, hbool_t minimize); * \since 1.0.0 * */ -H5_DLL herr_t H5Pset_external(hid_t plist_id, const char *name, off_t offset, hsize_t size); +H5_DLL herr_t H5Pset_external(hid_t plist_id, const char *name, HDoff_t offset, hsize_t size); /** * \ingroup DCPL * From 65650a36eda6707f3fd25decb2c667b06a887512 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Thu, 28 Mar 2024 11:00:12 -0700 Subject: [PATCH 2/2] Clean up HDoff_t PR * Move HDoff_t to H5public.h * Use typedefs instead of #define * Remove duplicated definitions of HDoff_t * Clean up h5_stat* definitions * Add Doxygen for HDoff_t --- release_docs/RELEASE.txt | 16 ++++++++++++++++ src/H5Ppublic.h | 14 -------------- src/H5private.h | 15 +++++++++------ src/H5public.h | 16 ++++++++++++++++ src/H5win32defs.h | 11 ++--------- 5 files changed, 43 insertions(+), 29 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 4e32cc7ef13..3a2f8fc1f87 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -178,6 +178,22 @@ New Features Library: -------- + - H5Pset_external() now uses HDoff_t, which is always a 64-bit type + + The H5Pset_external() call took an off_t parameter in HDF5 1.14.x and + earlier. On POSIX systems, off_t is specified as a 64-bit type via + POSIX large-file support (LFS). On Windows, however, off_t is defined + as a 32-bit type, even on 64-bit Windows. + + HDoff_t has been added to H5public.h and is defined to be __int64 on + Windows and the library has been updated to use HDoff_t in place of + off_t throughout. The H5Pset_external() offset parameter has also been + updated to be HDoff_t. + + There is no API compatibility wrapper for this change. + + Fixes GitHub issue #3506 + - Added support for in-place type conversion in most cases In-place type conversion allows the library to perform type conversion diff --git a/src/H5Ppublic.h b/src/H5Ppublic.h index 72d805e9a38..b4565fbdbde 100644 --- a/src/H5Ppublic.h +++ b/src/H5Ppublic.h @@ -5571,20 +5571,6 @@ H5_DLL herr_t H5Pget_mpi_params(hid_t fapl_id, MPI_Comm *comm, MPI_Info *info); H5_DLL herr_t H5Pset_mpi_params(hid_t fapl_id, MPI_Comm comm, MPI_Info info); #endif /* H5_HAVE_PARALLEL */ -#ifdef H5_HAVE_WIN32_API -/* off_t exists on Windows, but is always a 32-bit long, even on 64-bit Windows, - * so we define HDoff_t to be __int64, which is the type of the st_size field - * of the _stati64 struct. - */ -#define HDoff_t __int64 -#endif /* H5_HAVE_WIN32_API */ -#ifndef H5_HAVE_WIN32_API -/* These definitions differ in Windows and are defined in - * H5win32defs for that platform. - */ -#define HDoff_t off_t -#endif - /** * \ingroup FAPL * diff --git a/src/H5private.h b/src/H5private.h index 2a03e11720c..297bb2bd492 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -593,16 +593,19 @@ typedef struct { */ #include "H5win32defs.h" -/* Platform-independent definitions for struct stat and off_t */ -#ifndef H5_HAVE_WIN32_API -/* These definitions differ in Windows and are defined in - * H5win32defs for that platform. +/* Platform-independent definition for struct stat. For Win32, see + * H5win32defs.h. */ +#ifndef H5_HAVE_WIN32_API typedef struct stat h5_stat_t; -typedef off_t h5_stat_size_t; -#define HDoff_t off_t #endif +/* __int64 is the correct type for the st_size field of the _stati64 + * struct on Windows (MSDN isn't very clear about this). POSIX systems use + * off_t. Both of these are typedef'd to HDoff_t in H5public.h. + */ +typedef HDoff_t h5_stat_size_t; + /* Redefine all the POSIX and C functions. We should never see an * undecorated POSIX or C function (or any other non-HDF5 function) * in the source. diff --git a/src/H5public.h b/src/H5public.h index 08ef24d7938..56c2d71d75e 100644 --- a/src/H5public.h +++ b/src/H5public.h @@ -290,6 +290,22 @@ typedef long long ssize_t; */ typedef uint64_t hsize_t; +/* off_t exists on Windows, but is always a 32-bit long, even on 64-bit Windows, + * so on Windows we define HDoff_t to be __int64, which is the type of the + * st_size field of the _stati64 struct. + */ +#ifdef H5_HAVE_WIN32_API +/** + * Platform-independent offset + */ +typedef __int64 HDoff_t; +#else +/** + * Platform-independent offset + */ +typedef off_t HDoff_t; +#endif + #ifdef H5_HAVE_PARALLEL #define HSIZE_AS_MPI_TYPE MPI_UINT64_T #endif diff --git a/src/H5win32defs.h b/src/H5win32defs.h index ba6028ae9dd..1e43ed28cd5 100644 --- a/src/H5win32defs.h +++ b/src/H5win32defs.h @@ -20,17 +20,10 @@ #ifdef H5_HAVE_WIN32_API -/* off_t exists on Windows, but is always a 32-bit long, even on 64-bit Windows, - * so we define HDoff_t to be __int64, which is the type of the st_size field - * of the _stati64 struct. - */ -#define HDoff_t __int64 - -/* __int64 is the correct type for the st_size field of the _stati64 struct. - * MSDN isn't very clear about this. +/* Win32 platform-independent definition for struct stat. For POSIX, see + * H5private.h. */ typedef struct _stati64 h5_stat_t; -typedef __int64 h5_stat_size_t; #ifdef H5_HAVE_VISUAL_STUDIO struct timezone {