Skip to content

Emit HAVE__FSTAT/HAVE__FSTATI64 so fstat matches struct stat on Windows - #9259

Merged
joa-quim merged 2 commits into
GenericMappingTools:masterfrom
Sqhh99:fix-windows-fstat-struct
Oct 13, 2026
Merged

joa-quim merged 2 commits into
GenericMappingTools:masterfrom
Sqhh99:fix-windows-fstat-struct

Conversation

@Sqhh99

@Sqhh99 Sqhh99 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Description of proposed changes

ConfigureChecks.cmake checks for _fstat and _fstati64 on Windows, but gmt_config.h.in never writes HAVE__FSTAT or HAVE__FSTATI64 into the header. As a result, gmt_notposix.h maps stat to _stati64 (because HAVE__STATI64 is set) but leaves fstat unmapped. So in gmtlib_read_ps, struct stat buf becomes struct _stati64, while fstat() still expects the CRT's own struct stat, which has a different layout:

  • GCC 14+ (MinGW-w64) treats this as an error (-Wincompatible-pointer-types), as reported in mingw64 compiling failure #8825.
  • MSVC only warns, but fstat writes a struct of a different size than buf.

#8826 worked around this for 64-bit builds only, by calling _fstat64 under #ifdef _WIN64. 32-bit MinGW builds still fail, and 6.6.0 fails on both.

This PR adds the two missing #cmakedefine lines. fstat is then mapped to _fstati64, which pairs with _stati64 on every Windows target. That makes the _WIN64 special case unnecessary, so it is removed.

How it was tested

I used MinGW-w64 GCC 13 with -Werror=incompatible-pointer-types, which behaves like GCC 14:

  • A CMake configure under MinGW detects all four of HAVE__STAT, HAVE__STATI64, HAVE__FSTAT and HAVE__FSTATI64. The 6.7.0 Windows installer's gmt_config.h (an MSVC build) also defines HAVE__STAT and HAVE__STATI64.

  • I compiled src/gmt_plot.c against the generated headers:

    x86_64 i686
    master compiles (because of the _WIN64 workaround) error: passing argument 2 of 'fstat' from incompatible pointer type; expected 'struct stat *' but argument is of type 'struct _stati64 *'
    this PR compiles compiles

    With this PR the object files import matching stat/fstat pairs: _stat64/_fstat64 on x64 and _stati64/_fstati64 on i686.

  • gmtlib_read_ps is the only fstat caller. The checks only run inside if (WIN32), so non-Windows builds are unchanged.

Follow-up to #8825 / #8826.

The fix and its analysis were developed with help from Claude (AI assistant). I checked the analysis myself, and the MinGW x64/i686 build results above are from real compiles.

Reminders

  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • Add tests N/A: this is a MinGW/GCC 14 compile-time error that the test suite can't catch. GMT's Windows CI builds with MSVC only, and a MinGW CI job would be the real safeguard; I'm happy to propose one separately.
  • Describe changes to function behavior N/A: no function's behavior or arguments change.
  • Add documentation N/A: no new functionality.

ConfigureChecks.cmake tests for _fstat and _fstati64, but gmt_config.h.in
never emitted HAVE__FSTAT or HAVE__FSTATI64. As a result gmt_notposix.h
mapped stat to _stati64 but left fstat unmapped, so fstat() received a
struct _stati64 while expecting a struct stat with a 32-bit st_size:
a struct layout mismatch with MSVC and a hard error with GCC 14+
(-Wincompatible-pointer-types). Issue GenericMappingTools#8825 was worked around with an
_WIN64-only call to _fstat64 in gmtlib_read_ps; with the defines in place
the generic code is correct on all Windows builds, so the hack is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@joa-quim

Copy link
Copy Markdown
Member

Thanks. Let me find some time to tests this under a MSVC build.

@joa-quim joa-quim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, tested and run the tests locally (MSVC build) run OK.

@joa-quim
joa-quim merged commit 20d3b4b into GenericMappingTools:master Oct 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants