Skip to content
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

build: fix compiler warnings in feature detections #16287

Closed
wants to merge 74 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Feb 10, 2025

Fix or silence compiler warnings happening in feature detections
to reduce log noise. Warnings may also get promoted to errors in certain
cases, causing missed detections.

It reduces the number of warnings by 4500+ across the linux, linux-old,
macos, non-native and windows GHA workflows (~142 jobs).

Also move picky warning logic for MSVC/Borland to
`CMake/PickyWarnings.cmake. To make them listed in the picky-warnings
log output, and to also apply to feature detections to make them compile
under the same conditions as source code. The hope is to help catching
issues faster. It also improves code quality of feature tests.

Fixed/silenced:

warning #177: variable "dummy" was declared but never referenced
warning #177: variable "flag" was declared but never referenced
warning #177: variable "res" was declared but never referenced
warning #592: variable "s" is used before its value is set
warning #1011: missing return statement at end of non-void function "main"
warning #1786: function "SSL_CTX_set_srp_password" (declared at line 1888 of "/usr/include/openssl/ssl.h") was declared deprecated ("Since OpenSSL 3.0")
warning #1786: function "SSL_CTX_set_srp_username" (declared at line 1887 of "/usr/include/openssl/ssl.h") was declared deprecated ("Since OpenSSL 3.0")
warning #2332: a value of type "const char *" cannot be assigned to an entity of type "char *" (dropping qualifiers)
warning: 'SSL_CTX_set_srp_password' is deprecated [-Wdeprecated-declarations]
warning: 'SSL_CTX_set_srp_password' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
warning: 'SSL_CTX_set_srp_username' is deprecated [-Wdeprecated-declarations]
warning: 'SSL_CTX_set_srp_username' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
warning: 'b' is used uninitialized [-Wuninitialized]
warning: 'gethostname' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
warning: Undefined or garbage value returned to caller [core.uninitialized.UndefReturn]
warning: Value stored to 'i' is never read [deadcode.DeadStores]
warning: assigning to 'char *' from 'const char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
warning: control reaches end of non-void function [-Wreturn-type]
warning: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
warning: excess elements in struct initializer
warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
warning: macro "_FILE_OFFSET_BITS" is not used [-Wunused-macros]
warning: macro "_REENTRANT" is not used [-Wunused-macros]
warning: missing braces around initializer [-Wmissing-braces]
warning: no previous extern declaration for non-static variable 'off_t_is_large' [-Wmissing-variable-declarations]
warning: no previous prototype for 'check' [-Wmissing-prototypes]
warning: no previous prototype for function 'check' [-Wmissing-prototypes]
warning: null argument where non-null required (argument 2) [-Wnonnull]
warning: passing 'const char[1]' to parameter of type 'char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
warning: passing argument 2 of 'SSL_CTX_set_srp_password' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
warning: passing argument 2 of 'SSL_CTX_set_srp_username' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
warning: unused parameter 'c' [-Wunused-parameter]
warning: unused parameter 'f' [-Wunused-parameter]
warning: unused variable 'data' [-Wunused-variable]
warning: unused variable 'dummy' [-Wunused-variable]
warning: unused variable 'flag' [-Wunused-variable]
warning: unused variable 'res' [-Wunused-variable]
warning: unused variable 's' [-Wunused-variable]
warning: variable 's' set but not used [-Wunused-but-set-variable]
warning: variable 'ts' set but not used [-Wunused-but-set-variable]

w/o whitespace: https://github.com/curl/curl/pull/16287/files?w=1

Warnings possibly fixable, but not fixed in this PR:

ld: warning: ignoring duplicate libraries: '-lcrypto'
ld: warning: ignoring duplicate libraries: '-lcrypto', '-lnghttp2', '-lssl'
ld: warning: ignoring duplicate libraries: '-lcrypto', '-lrtmp', '-lssl'
ld: warning: ignoring duplicate libraries: '-lcrypto', '-lssl'

warning #1786: function "siginterrupt" (declared at line 324 of "/usr/include/signal.h") was declared deprecated ("Use sigaction with SA_RESTART instead")
warning: 'siginterrupt' is deprecated: Use sigaction with SA_RESTART instead [-Wdeprecated-declarations]

warning #2193: null argument provided for parameter marked with attribute "nonnull"
warning: Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]
warning: Null pointer passed to 2nd parameter expecting 'nonnull' [core.NonNullParamChecker]
warning: Null pointer passed to 3rd parameter expecting 'nonnull' [core.NonNullParamChecker]
warning: argument 1 null where non-null expected [-Wnonnull]
warning: argument 2 null where non-null expected [-Wnonnull]
warning: argument 3 null where non-null expected [-Wnonnull]
warning: null argument where non-null required (argument 1) [-Wnonnull]
warning: null passed to a callee that requires a non-null argument [-Wnonnull]

warning: 'ber_free' is deprecated: first deprecated in macOS 10.11 - use OpenDirectory Framework [-Wdeprecated-declarations]
warning: 'ber_init' is deprecated: first deprecated in macOS 10.11 - use OpenDirectory Framework [-Wdeprecated-declarations]
warning: 'ldap_init' is deprecated: first deprecated in macOS 10.10 - use ldap_initialize [-Wdeprecated-declarations]
warning: 'ldap_unbind' is deprecated: first deprecated in macOS 10.10 - use ldap_unbind_ext [-Wdeprecated-declarations]

warning: unused variable 'hdata' [-Wunused-variable]
warning: unused variable 'ts' [-Wunused-variable]

vszakats added a commit that referenced this pull request Feb 11, 2025
A deprecation error prevented correct detection in MSVC UWP builds:
```
curl\\bld\\CMakeFiles\\CMakeScratch\\TryCompile-ks2aa4\\CheckSymbolExists.c(8,19):
  error C4996: 'SSL_CTX_set_srp_username': Since OpenSSL 3.0
```
Ref: https://github.com/curl/curl/actions/runs/13242285473/job/36960223663#step:8:898

It seems to be caused by different default warning levels used by
the toolchain (or CMake?): `/W3` for UWP and `/W1` for Windows desktop.

https://github.com/curl/curl/actions/runs/13242285473/job/36960223663#step:8:893 UWP
https://github.com/curl/curl/actions/runs/13242285473/job/36960223262#step:8:445 desktop

Fix by passing the OpenSSL macro suppressing its deprecation warnings.

Cherry-picked from #16287
Closes #16293
@vszakats vszakats marked this pull request as draft February 11, 2025 22:39
@vszakats vszakats marked this pull request as ready for review February 13, 2025 14:55
@vszakats vszakats force-pushed the b-detwarn-2 branch 3 times, most recently from 32ea31a to a81bb2d Compare February 15, 2025 10:50
To make it listed in the picky-warning log message, and to apply to
feature detections, to make them more robust and compile under the
same conditions as the source code. The hope is this helps catching
issues faster. It also improves code quality of feature tests.
```
/cygdrive/d/a/curl/curl/CMake/CurlTests.c: In function 'check':
        /cygdrive/d/a/curl/curl/CMake/CurlTests.c:298:17: error: unused parameter 'c' [-Werror=unused-parameter]
          298 | void check(char c) {}
              |            ~~~~~^
```
https://github.com/curl/curl/actions/runs/13209220246/job/36879343197#step:7:4493

CurlTests.c: fix more compiler warnings
MSVC:
```
 D:\\a\\curl\\curl\\CMake\\CurlTests.c(171,17): warning C4189: 'flags': local variable is initialized but not referenced [D:\\a\\curl\\curl\\bld\\CMakeFiles\\CMakeTmp\\cmTC_41ff3.vcxproj]
```
https://github.com/curl/curl/actions/runs/13215270720/job/36893785298?pr=16238#step:7:7204
```
D:\\a\\curl\\curl\\CMake\\CurlTests.c(133,6): warning C4127: conditional expression is constant [D:\\a\\curl\\curl\\bld\\CMakeFiles\\CMakeTmp\\cmTC_1d26c.vcxproj]
```
https://github.com/curl/curl/actions/runs/13216319022/job/36896009461?pr=16238#step:8:7888
```
D:/a/curl/curl/CMake/CurlTests.c:286:3: warning: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
```
https://github.com/curl/curl/actions/runs/13226709122/job/36921490343?pr=16278
```
D:/a/curl/curl/CMake/CurlTests.c:154:5: warning: no previous extern declaration for non-static variable 'off_t_is_large' [-Wmissing-variable-declarations]
  154 | int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
      |     ^
D:/a/curl/curl/CMake/CurlTests.c:154:1: note: declare 'static' if the variable is not intended to be used outside of this translation unit
  154 | int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
      | ^
```
https://github.com/curl/curl/actions/runs/13226709122/job/36921490343?pr=16278#step:8:3911
```
        /d/a/curl/curl/CMake/CurlTests.c:80:23: warning: unused variable ‘hdata’ [-Wunused-variable]
           80 |   struct hostent_data hdata;
              |                       ^~~~~
```
https://github.com/curl/curl/actions/runs/13226709122/job/36921489357?pr=16278#step:8:3941 (msys2)
https://github.com/curl/curl/actions/runs/13226709122/job/36921488213?pr=16278#step:7:3952 (cygwin)
mingw gcc:
```
        D:/a/curl/curl/CMake/CurlTests.c:61:12: error: "_REENTRANT" redefined
           61 | #   define _REENTRANT
              |            ^~~~~~~~~~
        <command-line>: note: this is the location of the previous definition
```
https://github.com/curl/curl/actions/runs/13226709122/job/36921490162?pr=16278#step:8:3751
```
D:/a/curl/curl/CMake/CurlTests.c:155:12: warning: unused variable 'off_t_is_large' [-Wunused-variable]
```
This reverts commit 6f0551d.

AM IntelC openssl.txt:conftest.c(61):
```
warning curl#592: variable "s" is used before its value is set
```
```
conftest.c:161:38: warning: redundant redeclaration of 'gethostname' [-Wredundant-decls]
  161 |               extern int FUNCALLCONV gethostname(char *, int);
      |                                      ^~~~~~~~~~~
In file included from conftest.c:127:
D:/a/_temp/msys64/mingw64/include/winsock2.h:1040:34: note: previous declaration of 'gethostname' with type 'int(char *, int)'
 1040 |   WINSOCK_API_LINKAGE int WSAAPI gethostname(char *name,int namelen);
      |                                  ^~~~~~~~~~~
conftest.c:161:38: warning: 'gethostname' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
  161 |               extern int FUNCALLCONV gethostname(char *, int);
      |                                      ^~~~~~~~~~~
```
…ixup 1

AmigaOS:
```
warning: excess elements in struct initializer
```
…inter target type [-Wdiscarded-qualifiers] (AmigaOS)
```
conftest.c(38): error: #error directive: force compilation error
          #error force compilation error
           ^

conftest.c(40): warning curl#1011: missing return statement at end of non-void function "main"
        }
        ^
```
MS-DOS:
```
/home/runner/work/curl/curl/CMake/CurlTests.c:324:25: warning: excess elements in struct initializer
  324 |   struct timespec ts = {0, 0};
      |                         ^
```

AmigaOS:
```
/home/runner/work/curl/curl/CMake/CurlTests.c:324:24: warning: missing braces around initializer [-Wmissing-braces]
   struct timespec ts = {0, 0};
                        ^
```
This allows to avoid the unused variable warning when the struct is
not available.
@vszakats vszakats closed this in ca2f49d Feb 16, 2025
@vszakats vszakats deleted the b-detwarn-2 branch February 16, 2025 01:41
vszakats added a commit that referenced this pull request Feb 21, 2025
Silence compiler warnings (200 of them across the main CI workflows):
```
warning #2193: null argument provided for parameter marked with attribute "nonnull"
warning: Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]
warning: Null pointer passed to 2nd parameter expecting 'nonnull' [core.NonNullParamChecker]
warning: argument 1 null where non-null expected [-Wnonnull]
warning: argument 2 null where non-null expected [-Wnonnull]
warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits]
warning: null argument where non-null required (argument 1) [-Wnonnull]
```

Also drop `if ... can be linked` feature checks that were identical to
`if ... is compilable` checks, for:
`closesocket`, `ioctlsocket`, `socket`, `freeaddrinfo`, `getaddrinfo`,
`gethostname`, `getpeername`, `getsockname`,
`CloseSocket` (AmigaOS), `IoctlSocket` (AmigaOS).
Another option is to really do the link checks. But, if they weren't
missed so far, it seems safer to drop than risk a detection failure,
as was the case with AmigaOS functions while working on this PR.

There remain 22 `-Wnonnull` warnings in `gethostbyname_r()`,
`getpeername()` `getsockname()`. Most of the rest is necessary for
detection, or originate from autotools and CMake detection code
templates. Some still fixable, like duplicate libs.

Follow-up to ca2f49d #16287
Closes #16377
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant