-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lib: set more flags in config-win32.h #9712
Conversation
The AppVeyor CI job failure is unrelated:
https://ci.appveyor.com/project/curlorg/curl/builds/45050447/job/odv664nav6sytial#L7280 |
The goal is to add any flag that affect the created binary, to get in sync with the ones built with CMake and autotools. I took these flags from curl-for-win [0], where they've been tested with mingw-w64 and proven to work well. This patch brings them to curl as follows: - Enable unconditionally those force-enabled via `CMake/WindowsCache.cmake`: - `HAVE_SETJMP_H` - `HAVE_STRING_H` - `HAVE_SIGNAL` (CMake equivalent is `HAVE_SIGNAL_FUNC`) - Expand existing guards with mingw-w64: - `HAVE_STDBOOL_H` - `HAVE_BOOL_T` - Enable Win32 API functions for Windows Vista and later: - `HAVE_INET_NTOP` - `HAVE_INET_PTON` - Set sizes, if not already set: - `SIZEOF_OFF_T = 8` - `_FILE_OFFSET_BITS = 64` when `USE_WIN32_LARGE_FILES` is set, and using mingw-w64. - Add the remaining for mingw-w64 only. Feel free to expand as desired: - `HAVE_LIBGEN_H` - `HAVE_FTRUNCATE` - `HAVE_BASENAME` - `HAVE_STRTOK_R` Future TODOs: - `HAVE_SIGNAL` has a different meaning in CMake. It's enabled when both the `signal()` function and the `SIGALRM` macro are found. In autotools and this header, it means the function only. For the function alone, CMake uses `HAVE_SIGNAL_FUNC`. [0] https://github.com/curl/curl-for-win/blob/c9b9a5f273c94c73d2b565ee892c4dff0ca97a8c/curl-m32.sh#L53-L58 Closes #xxxx
Wow, that was a spectacularly weird error! |
`__MINGW32__` is always defined when `__MINGW64_VERSION_MAJOR` is.
@bagder, @vszakats , building libcurl for windows (64bit with VS2022), it appears sizeof(off_t) is equal to 4 and not 8. Looking at the CRT's types.h file (Windows 10 SDK 10.0.19041.0), off_t is defined as After updating from 7.84.0 to 7.86.0, a static_assert in our code was triggered, which we added among others to make sure libcurl was not misconfigured for the target: I was also surprised to find that off_t and SIZEOF_OFF_T appear to be not used anywhere inside libcurl, and I was wondering why it was added in the first place if not used. |
@PeterPiekarski Thanks for your report. Before this patch this macro settled on 4 by default, indeed. I didn't recognize this type is CRT-dependent. So maybe this method would be better by limiting this line to MinGW: #if defined(__MINGW32__)
# define SIZEOF_OFF_T 8
#endif As a temporary workaround you can use Oops, forgot to add: This value is used in curl, in |
The correct value for MSVC is 4. Let's limit the value 8 for MinGW only. This lets other Windows compilers fall back to the default value of 4. Regression in 7.86.0 (from 68fa9bf) Bug: curl#9712 (comment) Reported-by: Peter Piekarski Closes #xxxx
The previously set default value of 8 (64-bit) is only correct for mingw-w64 and only when we set `_FILE_OFFSET_BITS` to 64 (the default when building curl). For MSVC, old MinGW and other Windows compilers, the correct value is 4 (32-bit). Adjust condition accordingly. Also drop the manual override option. Regression in 7.86.0 (from 68fa9bf) Bug: #9712 (comment) Reported-by: Peter Piekarski Reviewed-by: Jay Satiro Closes #9872
The goal is to add any flag that affect the created binary, to get in sync with the ones built with CMake and autotools.
I took these flags from curl-for-win [0], where they've been tested with mingw-w64 and proven to work well.
This patch brings them to curl as follows:
Enable unconditionally those force-enabled via
CMake/WindowsCache.cmake
:HAVE_SETJMP_H
HAVE_STRING_H
HAVE_SIGNAL
(CMake equivalent isHAVE_SIGNAL_FUNC
)Expand existing guards with mingw-w64:
HAVE_STDBOOL_H
HAVE_BOOL_T
Enable Win32 API functions for Windows Vista and later:
HAVE_INET_NTOP
HAVE_INET_PTON
Set sizes, if not already set:
SIZEOF_OFF_T = 8
_FILE_OFFSET_BITS = 64
whenUSE_WIN32_LARGE_FILES
is set,and using mingw-w64.
Add the remaining for mingw-w64 only. Feel free to expand as desired:
HAVE_LIBGEN_H
HAVE_FTRUNCATE
HAVE_BASENAME
HAVE_STRTOK_R
Future TODOs:
HAVE_SIGNAL
has a different meaning in CMake. It's enabled when both thesignal()
function and theSIGALRM
macro are found. In autotools and this header, it means the function only. For the function alone, CMake usesHAVE_SIGNAL_FUNC
.[0] https://github.com/curl/curl-for-win/blob/c9b9a5f273c94c73d2b565ee892c4dff0ca97a8c/curl-m32.sh#L53-L58
Closes #xxxx