lib: fix warnings about undefined macros #1362
Conversation
|
@MarcelRaad, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bagder, @yangtse and @captain-caveman2k to be potential reviewers. |
|
I think we should require for all build systems either CURL_SIZEOF_LONG or SIZEOF_LONG and whichever one is required outlaw the other. Same for TIME_T. |
|
Yes, I've verified that But I find it a bit strange that |
|
Symbols defined in our public header files must be "curl" prefixed, so if it is to be there it must be |
lib/curl_ntlm_core.h
Outdated
| @@ -53,6 +53,10 @@ | |||
| #define USE_NTLM2SESSION 1 | |||
| #endif | |||
|
|
|||
| #if !defined(USE_NTLM2SESSION) | |||
| #define USE_NTLM2SESSION 0 | |||
There was a problem hiding this comment.
So what's the exact warning this fixes? I only see a !defined(USE_NTLM2SESSION) use of this and that looks perfectly fine to me?
There was a problem hiding this comment.
Depending on how libcurl is built with ntlm USE_NTLM2SESSION is defined 0 or 1 or undefined. In ntlm code I notice there is a lot of #if FOO instead of #if defined(FOO). For example there is this twice:
#if USE_NTRESPONSES && USE_NTLM2SESSION
He has a compiler switch that catches when FOO macro is evaluated to 0 due to undefined FOO
There was a problem hiding this comment.
I would prefer that we rather make sure to depend on features with #ifdef so that we #undef features we don't have and we #define those we have. I think that's the pattern used more widely already. Defining to zero when we don't have it is an anomaly.
There was a problem hiding this comment.
Yes, exactly. All the macros defined in curl_ntlm_core.h and used in curl_ntlm_core.c and vauth/ntlm.c are checked with #if instead of #ifdef:
USE_NTLM_V2
USE_NTRESPONSES
USE_NTLM2SESSION
Seems like this is for historical reasons, they were always defined to 1 when introduced in 2006.
I'll glady change this in a separate pull request and remove the curl_ntlm_core.h changes from this one.
| @@ -164,7 +164,7 @@ int Curl_socket_check(curl_socket_t readfd0, /* two sockets to read from */ | |||
| int r; | |||
| int ret; | |||
|
|
|||
| #if SIZEOF_LONG != SIZEOF_INT | |||
| #if SIZEOF_TIME_T != SIZEOF_INT | |||
Feature defines are normally checked with #ifdef instead of #if in the rest of the codebase. Additionally, some compilers warn when a macro is implicitly evaluated to 0 because it is not defined, which was the case here. Ref: curl#1362 (comment)
Feature defines are normally checked with #ifdef instead of #if in the rest of the codebase. Additionally, some compilers warn when a macro is implicitly evaluated to 0 because it is not defined, which was the case here. Ref: curl#1362 (comment) Closes curl#1367
MarcelRaad
force-pushed
the
fix_undefined_macros
branch
from
9ed689f to
0c460a4
Compare
At least under Windows, there is no SIZEOF_LONG, so it evaluates to 0 even though sizeof(int) == sizeof(long). This should probably have been CURL_SIZEOF_LONG, but the type of timeout_ms changed from long to time_t anyway. This triggered MSVC warning C4668 about implicitly replacing undefined macros with '0'. Closes curl#1362
MarcelRaad
force-pushed
the
fix_undefined_macros
branch
from
0c460a4 to
1ff689a
Compare
|
Updated and rebased after merging #1367 . Now only the change from SIZEOF_LONG to SIZEOF_TIME_T remains. |
At least under Windows, there is no
SIZEOF_LONG, so it evaluates to 0 eventhough
sizeof(int) == sizeof(long). This should probably have beenCURL_SIZEOF_LONG, but the type oftimeout_mschanged fromlongtotime_tanyway.
Also defaulted
USE_NTLM2SESSIONto 0 if it's not defined so that MSVC warningC4668 about implicitly replacing undefined macros with '0' can now be enabled.