Skip to content

Commit

Permalink
Change type of offset arg in H5Pset_external to HDoff_t (#3505)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
phil-opp authored Jun 4, 2024
1 parent 552510e commit b698b48
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 20 deletions.
1 change: 1 addition & 0 deletions bin/trace
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,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",
Expand Down
16 changes: 16 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,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

- Relaxed behavior of H5Pset_page_buffer_size() when opening files

This API call sets the size of a file's page buffer cache. This call
Expand Down
9 changes: 5 additions & 4 deletions src/H5Pdcpl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2586,7 +2586,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;
Expand Down Expand Up @@ -2616,8 +2616,8 @@ H5Pset_external(hid_t plist_id, const char *name, off_t offset, hsize_t size)
tmp = total + efl.slot[idx].size;
if (tmp <= total)
HGOTO_ERROR(H5E_EFL, H5E_OVERFLOW, FAIL, "total external data size overflowed");
} /* end for */
} /* end if */
}
}

/* Add to the list */
if (efl.nused >= efl.nalloc) {
Expand All @@ -2628,7 +2628,8 @@ H5Pset_external(hid_t plist_id, const char *name, off_t offset, hsize_t size)
HGOTO_ERROR(H5E_RESOURCE, H5E_CANTALLOC, FAIL, "memory allocation failed");
efl.nalloc = na;
efl.slot = x;
} /* end if */
}

idx = efl.nused;
efl.slot[idx].name_offset = 0; /*not entered into heap yet*/
efl.slot[idx].name = H5MM_xstrdup(name);
Expand Down
3 changes: 2 additions & 1 deletion src/H5Ppublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -5678,6 +5678,7 @@ 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 */

/**
* \ingroup FAPL
*
Expand Down Expand Up @@ -6532,7 +6533,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
*
Expand Down
15 changes: 9 additions & 6 deletions src/H5private.h
Original file line number Diff line number Diff line change
Expand Up @@ -657,16 +657,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;

/* Redefinions of some POSIX and C functions (mainly to deal with Windows) */

#ifndef HDaccess
Expand Down
16 changes: 16 additions & 0 deletions src/H5public.h
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,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
Expand Down
11 changes: 2 additions & 9 deletions src/H5win32defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 and what is returned by _ftelli64().
*/
#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 {
Expand Down

0 comments on commit b698b48

Please sign in to comment.