Skip to contentSkip to conversation

win32: ensure that `localtime_r()` is declared even in i686 builds

PATCH In review Johannes Schindelin via GitGitGadget 2 participants 2 messages Jun 22

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 win32: ensure that `localtime_r()` is declared even in i686 builds

From: Johannes Schindelin <[email protected]>

The __MINGW64__ constant is defined, surprise, surprise, only when building for a 64-bit CPU architecture.

Therefore using it as a guard to define _POSIX_C_SOURCE (so that localtime_r() is declared, among other functions) is not enough, we also need to check __MINGW32__.

Technically, the latter constant is defined even for 64-bit builds. But let's make things a bit easier to understand by testing for both constants.

Making it so fixes this compile warning (turned error in GCC v14.1):

archive-zip.c: In function 'dos_time':
archive-zip.c:612:9: error: implicit declaration of function 'localtime_r'; did you mean 'localtime_s'? [-Wimplicit-function-declaration]

    612 |         localtime_r(&time, &tm);
        |         ^~~~~~~~~~~
        |         localtime_s

---
win32: ensure that localtime_r() is declared even in i686 builds

Git for Windows plans on reducing the scope of its i686 support after v2.55.0 even further, therefore this patch (which I had forgotten about) needs to be in that version.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2157%2Fdscho%2FFix-i686-build-with-GCC-v14-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2157/dscho/Fix-i686-build-with-GCC-v14-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2157

 compat/posix.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
compat/posix.h +1 -1
index 2f01564b0d..e2e794cad7 100644
@@ -56,7 +56,7 @@
 # define UNUSED
 #endif
 
-#ifdef __MINGW64__
+#if defined(__MINGW32__) || defined(__MINGW64__)
 #define _POSIX_C_SOURCE 1
 #elif defined(__sun__)
  /*

base-commit: 8d96f09e9245ddf80c1981476fcbac8c4bb4125f

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 5 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The __MINGW64__ constant is defined, surprise, surprise, only when building for a 64-bit CPU architecture.

Quoted 7 lines

Therefore using it as a guard to define _POSIX_C_SOURCE (so that localtime_r() is declared, among other functions) is not enough, we also need to check __MINGW32__.

Technically, the latter constant is defined even for 64-bit builds. But let's make things a bit easier to understand by testing for both constants.

So it would suffice to use __MINGW32__? In any case, I agree that making this explicit feels sane.

Quoted 8 lines

Making it so fixes this compile warning (turned error in GCC v14.1):

archive-zip.c: In function 'dos_time':
archive-zip.c:612:9: error: implicit declaration of function 'localtime_r'; did you mean 'localtime_s'? [-Wimplicit-function-declaration]

    612 |         localtime_r(&time, &tm);
        |         ^~~~~~~~~~~
        |         localtime_s

Makes sense. The function is available in C23, but we don't use it. Otherwise, it's enabled with _POSIX_C_SOURCE according to [1].

compat/posix.h line 62

Quoted 13 lines
compat/posix.h +1 -1
index 2f01564b0d..e2e794cad7 100644
@@ -56,7 +56,7 @@
 # define UNUSED
 #endif
 
-#ifdef __MINGW64__
+#if defined(__MINGW32__) || defined(__MINGW64__)
 #define _POSIX_C_SOURCE 1
 #elif defined(__sun__)
  /*

This looks nice and simple.

Thanks!

Patrick

[1]: https://man7.org/linux/man-pages/man3/ctime.3.html