Skip to content

cmake: fix clang-tidy builds to verify tests, fix fallouts #16756

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

Closed
wants to merge 85 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 17, 2025

  • cmake: disable test bundles for clang-tidy builds.
    clang-tidy ignores #included .c sources, and incompatible with unity
    and bundles. It caused clang-tidy ignoring all test sources. It also
    means this is the first time tests sources are checked with
    clang-tidy. (autotools doesn't run it on tests.)

  • cmake: update description for CURL_TEST_BUNDLES option.

  • fix tests using special CURLE_* enums that were missing from
    curl/curl.h. Add them as reserved codes.

  • fix about ~50 other issues detected by clang-tidy: unchecked results,
    NULL derefs, memory leaks, casts to enums, unused assigments,
    uninitialized errno uses, unchecked open, indent, and more.

  • drop unnecessary casts (lib1533, lib3207).

  • suppress a few impossible cases with detailed NOLINTs.

  • lib/escape.c: drop NOLINT no longer necessary.
    Follow-up to 72abf7c lib: tidy up types and casts #13862 (possibly)

  • extend two existing NOLINT comments with details.

Follow-up to fabfa8e #15825


w/o ws https://github.com/curl/curl/pull/16756/files?w=1

  • why is clang-tidy silent with the test bundle sources?
    https://github.com/curl/curl/actions/runs/13922014255/job/38957321589#step:14:293
    According to isolated local tests, it ignores #include "source.c"-ed sources. Also when
    invoking with the --system-headers option.
    This means it's also not compatible with unity builds.
    Actually CMake explicitly excludes included sources from clang-tidy with these comments:
    /* generated by CMake */
    
    /* NOLINTNEXTLINE(bugprone-suspicious-include,misc-include-cleaner) */
    #include "[...]/lib/altsvc.c"
    Possibly for old clang-tidy versions that weren't doing this automatically? But why do this in the first place? Too much false positives or performance reasons?

@vszakats vszakats added the build label Mar 17, 2025
@github-actions github-actions bot added tests CI Continuous Integration labels Mar 17, 2025
@vszakats

This comment was marked as resolved.

@vszakats vszakats changed the title libauthretry: silence clang-tidy warnings tests: fix clang-tidy warnings Mar 18, 2025
@vszakats vszakats force-pushed the tidyenum branch 2 times, most recently from 1e62535 to d0bef8e Compare March 18, 2025 03:25
Seen when running the macOS clang-tidy job with test bundles disabled:
```
tests/libtest/libauthretry.c:96:12: error: The value '126' provided to the cast
  expression is not in the valid range of values for the enum
  [clang-analyzer-optin.core.EnumCastOutOfRange,-warnings-as-errors]
   96 |     return TEST_ERR_MAJOR_BAD;
      |            ^
tests/libtest/test.h:99:32: note: expanded from macro 'TEST_ERR_MAJOR_BAD'
   99 | #define TEST_ERR_MAJOR_BAD     (CURLcode) 126
      |                                ^~~~~~~~~~~~~~
include/curl/curl.h:519:9: note: enum declared here
[...]
```
Ref: https://github.com/curl/curl/actions/runs/13909934306/job/38921798619?pr=16750#step:14:384

Follow-up to fabfa8e curl#15825
vszakats added 14 commits March 18, 2025 16:06
```
../../../tests/libtest/lib1559.c:39:3: error: variable 'res' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
   39 |   global_init(CURL_GLOBAL_ALL);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
https://github.com/curl/curl/actions/runs/13926157184/job/38971131476?pr=16756#step:39:63
```
tests/server/sws.c:782:40: error: An undefined value may be read from 'errno' [clang-analyzer-unix.Errno,-warnings-as-errors]
  782 |   } while((writeleft > 0) && ((error = errno) == EINTR));
      |                                        ^
tests/server/sws.c:2033:9: note: Assuming 'argc' is <= 'arg'
 2033 |   while(argc > arg) {
      |         ^~~~~~~~~~
tests/server/sws.c:2033:3: note: Loop condition is false. Execution continues on line 2175
 2033 |   while(argc > arg) {
      |   ^
tests/server/sws.c:2177:13: note: 'is_proxy' is false
 2177 |             is_proxy ? "-proxy" : "", socket_type);
      |             ^~~~~~~~
tests/server/sws.c:2177:13: note: '?' condition is false
tests/server/sws.c:2187:6: note: Assuming 'req' is non-null
 2187 |   if(!req)
      |      ^~~~
tests/server/sws.c:2187:3: note: Taking false branch
 2187 |   if(!req)
      |   ^
tests/server/sws.c:2195:6: note: Assuming the condition is false
 2195 |   if(CURL_SOCKET_BAD == sock) {
      |      ^
include/curl/curl.h:145:25: note: expanded from macro 'CURL_SOCKET_BAD'
  145 | #define CURL_SOCKET_BAD -1
      |                         ^
tests/server/sws.c:2195:3: note: Taking false branch
 2195 |   if(CURL_SOCKET_BAD == sock) {
      |   ^
tests/server/sws.c:2202:6: note: Assuming the condition is false
 2202 |   if(0 != setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2203 |                      (void *)&flag, sizeof(flag))) {
      |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:2202:3: note: Taking false branch
 2202 |   if(0 != setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
      |   ^
tests/server/sws.c:2209:6: note: Assuming the condition is false
 2209 |   if(0 != curlx_nonblock(sock, TRUE)) {
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:2209:3: note: Taking false branch
 2209 |   if(0 != curlx_nonblock(sock, TRUE)) {
      |   ^
tests/server/sws.c:2216:3: note: 'Default' branch taken. Execution continues on line 2238
 2216 |   switch(socket_domain) {
      |   ^
tests/server/sws.c:2238:11: note: 'rc' is equal to 0
 2238 |   if(0 != rc) {
      |           ^~
tests/server/sws.c:2238:3: note: Taking false branch
 2238 |   if(0 != rc) {
      |   ^
tests/server/sws.c:2251:7: note: 'port' is 8999
 2251 |   if(!port) {
      |       ^~~~
tests/server/sws.c:2251:3: note: Taking false branch
 2251 |   if(!port) {
      |   ^
tests/server/sws.c:2295:6: note: 'socket_domain' is not equal to AF_UNIX
 2295 |   if(socket_domain != AF_UNIX)
      |      ^~~~~~~~~~~~~
tests/server/sws.c:2295:3: note: Taking true branch
 2295 |   if(socket_domain != AF_UNIX)
      |   ^
tests/server/sws.c:2304:6: note: Assuming 'rc' is equal to 0
 2304 |   if(0 != rc) {
      |      ^~~~~~~
tests/server/sws.c:2304:3: note: Taking false branch
 2304 |   if(0 != rc) {
      |   ^
tests/server/sws.c:2321:6: note: Assuming 'wrotepidfile' is not equal to 0
 2321 |   if(!wrotepidfile)
      |      ^~~~~~~~~~~~~
tests/server/sws.c:2321:3: note: Taking false branch
 2321 |   if(!wrotepidfile)
      |   ^
tests/server/sws.c:2325:6: note: Assuming 'wroteportfile' is not equal to 0
 2325 |   if(!wroteportfile)
      |      ^~~~~~~~~~~~~~
tests/server/sws.c:2325:3: note: Taking false branch
 2325 |   if(!wroteportfile)
      |   ^
tests/server/sws.c:2334:3: note: Loop condition is true.  Entering loop body
 2334 |   for(;;) {
      |   ^
tests/server/sws.c:2343:39: note: Assuming 'socket_idx' is < 1
 2343 |     for(socket_idx = num_sockets - 1; socket_idx >= 1; --socket_idx) {
      |                                       ^~~~~~~~~~~~~~~
tests/server/sws.c:2343:5: note: Loop condition is false. Execution continues on line 2353
 2343 |     for(socket_idx = num_sockets - 1; socket_idx >= 1; --socket_idx) {
      |     ^
tests/server/sws.c:2353:8: note: Assuming 'got_exit_signal' is 0
 2353 |     if(got_exit_signal)
      |        ^~~~~~~~~~~~~~~
tests/server/sws.c:2353:5: note: Taking false branch
 2353 |     if(got_exit_signal)
      |     ^
tests/server/sws.c:2360:5: note: Loop condition is true.  Entering loop body
 2360 |     for(socket_idx = 0; socket_idx < num_sockets; ++socket_idx) {
      |     ^
tests/server/sws.c:2370:10: note: Assuming the condition is false
 2370 |       if(all_sockets[socket_idx] > maxfd)
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:2370:7: note: Taking false branch
 2370 |       if(all_sockets[socket_idx] > maxfd)
      |       ^
tests/server/sws.c:2360:5: note: Loop condition is false. Execution continues on line 2374
 2360 |     for(socket_idx = 0; socket_idx < num_sockets; ++socket_idx) {
      |     ^
tests/server/sws.c:2374:8: note: 'got_exit_signal' is 0
 2374 |     if(got_exit_signal)
      |        ^~~~~~~~~~~~~~~
tests/server/sws.c:2374:5: note: Taking false branch
 2374 |     if(got_exit_signal)
      |     ^
tests/server/sws.c:2379:13: note: Assuming 'rc' is >= 0
 2379 |     } while(rc < 0 && SOCKERRNO == SOCKEINTR && !got_exit_signal);
      |             ^~~~~~
tests/server/sws.c:2379:20: note: Left side of '&&' is false
 2379 |     } while(rc < 0 && SOCKERRNO == SOCKEINTR && !got_exit_signal);
      |                    ^
tests/server/sws.c:2377:5: note: Loop condition is false.  Exiting loop
 2377 |     do {
      |     ^
tests/server/sws.c:2381:8: note: 'got_exit_signal' is 0
 2381 |     if(got_exit_signal)
      |        ^~~~~~~~~~~~~~~
tests/server/sws.c:2381:5: note: Taking false branch
 2381 |     if(got_exit_signal)
      |     ^
tests/server/sws.c:2384:8: note: 'rc' is >= 0
 2384 |     if(rc < 0) {
      |        ^~
tests/server/sws.c:2384:5: note: Taking false branch
 2384 |     if(rc < 0) {
      |     ^
tests/server/sws.c:2390:8: note: Assuming 'rc' is not equal to 0
 2390 |     if(rc == 0) {
      |        ^~~~~~~
tests/server/sws.c:2390:5: note: Taking false branch
 2390 |     if(rc == 0) {
      |     ^
tests/server/sws.c:2397:8: note: Assuming the condition is true
 2397 |     if(FD_ISSET(all_sockets[0], &input)) {
      |        ^
tests/server/sws.c:2397:5: note: Taking true branch
 2397 |     if(FD_ISSET(all_sockets[0], &input)) {
      |     ^
tests/server/sws.c:2404:9: note: Taking false branch
 2404 |         if(CURL_SOCKET_BAD == msgsock)
      |         ^
tests/server/sws.c:2406:17: note: Field 'delay' is 0
 2406 |         if(req->delay)
      |                 ^
tests/server/sws.c:2406:9: note: Taking false branch
 2406 |         if(req->delay)
      |         ^
tests/server/sws.c:2400:7: note: Loop condition is false.  Exiting loop
 2400 |       do {
      |       ^
tests/server/sws.c:2413:26: note: Assuming 'socket_idx' is < 'num_sockets'
 2413 |     for(socket_idx = 1; (socket_idx < num_sockets) && active; ++socket_idx) {
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:2413:25: note: Left side of '&&' is true
 2413 |     for(socket_idx = 1; (socket_idx < num_sockets) && active; ++socket_idx) {
      |                         ^
tests/server/sws.c:2413:5: note: Loop condition is true.  Entering loop body
 2413 |     for(socket_idx = 1; (socket_idx < num_sockets) && active; ++socket_idx) {
      |     ^
tests/server/sws.c:2414:10: note: Assuming the condition is true
 2414 |       if(FD_ISSET(all_sockets[socket_idx], &input)) {
      |          ^
tests/server/sws.c:2414:7: note: Taking true branch
 2414 |       if(FD_ISSET(all_sockets[socket_idx], &input)) {
      |       ^
tests/server/sws.c:2416:12: note: Assuming 'got_exit_signal' is 0
 2416 |         if(got_exit_signal)
      |            ^~~~~~~~~~~~~~~
tests/server/sws.c:2416:9: note: Taking false branch
 2416 |         if(got_exit_signal)
      |         ^
tests/server/sws.c:2421:16: note: Calling 'service_connection'
 2421 |           rc = service_connection(all_sockets[socket_idx], req, sock,
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2422 |                                   connecthost, keepalive_secs);
      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:1934:6: note: 'got_exit_signal' is 0
 1934 |   if(got_exit_signal)
      |      ^~~~~~~~~~~~~~~
tests/server/sws.c:1934:3: note: Taking false branch
 1934 |   if(got_exit_signal)
      |   ^
tests/server/sws.c:1937:3: note: Loop condition is true.  Entering loop body
 1937 |   while(!req->done_processing) {
      |   ^
tests/server/sws.c:1938:14: note: Calling 'sws_get_request'
 1938 |     int rc = sws_get_request(msgsock, req);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:839:11: note: Field 'upgrade_request' is false
  839 |   if(req->upgrade_request) {
      |           ^
tests/server/sws.c:839:3: note: Taking false branch
  839 |   if(req->upgrade_request) {
      |   ^
tests/server/sws.c:904:3: note: Taking false branch
  904 |   if(req->offset >= REQBUFSIZ-1) {
      |   ^
tests/server/sws.c:909:13: note: Field 'skip' is 0
  909 |     if(req->skip)
      |             ^
tests/server/sws.c:909:5: note: Taking false branch
  909 |     if(req->skip)
      |     ^
tests/server/sws.c:917:8: note: 'got_exit_signal' is 0
  917 |     if(got_exit_signal)
      |        ^~~~~~~~~~~~~~~
tests/server/sws.c:917:5: note: Taking false branch
  917 |     if(got_exit_signal)
      |     ^
tests/server/sws.c:919:8: note: Assuming 'got' is equal to 0
  919 |     if(got == 0) {
      |        ^~~~~~~~
tests/server/sws.c:919:5: note: Taking true branch
  919 |     if(got == 0) {
      |     ^
tests/server/sws.c:932:8: note: 'fail' is 1
  932 |     if(fail) {
      |        ^~~~
tests/server/sws.c:932:5: note: Taking true branch
  932 |     if(fail) {
      |     ^
tests/server/sws.c:935:7: note: Calling 'sws_storerequest'
  935 |       sws_storerequest(reqbuf, req->offset);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:755:21: note: Assuming 'is_proxy' is false
  755 |             logdir, is_proxy ? REQUEST_PROXY_DUMP : REQUEST_DUMP);
      |                     ^~~~~~~~
tests/server/sws.c:755:21: note: '?' condition is false
tests/server/sws.c:757:7: note: 'reqbuf' is non-null
  757 |   if(!reqbuf)
      |       ^~~~~~
tests/server/sws.c:757:3: note: Taking false branch
  757 |   if(!reqbuf)
      |   ^
tests/server/sws.c:759:6: note: Assuming 'totalsize' is not equal to 0
  759 |   if(totalsize == 0)
      |      ^~~~~~~~~~~~~~
tests/server/sws.c:759:3: note: Taking false branch
  759 |   if(totalsize == 0)
      |   ^
tests/server/sws.c:765:11: note: Assuming 'dump' is non-null
  765 |   } while(!dump && ((error = errno) == EINTR));
      |           ^~~~~
tests/server/sws.c:765:17: note: Left side of '&&' is false
  765 |   } while(!dump && ((error = errno) == EINTR));
      |                 ^
tests/server/sws.c:762:3: note: Loop condition is false.  Exiting loop
  762 |   do {
      |   ^
tests/server/sws.c:766:7: note: 'dump' is non-null
  766 |   if(!dump) {
      |       ^~~~
tests/server/sws.c:766:3: note: Taking false branch
  766 |   if(!dump) {
      |   ^
tests/server/sws.c:775:15: note: Assuming that 'fwrite' is successful; 'errno' becomes undefined after the call
  775 |     written = fwrite(&reqbuf[totalsize-writeleft],
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  776 |                      1, writeleft, dump);
      |                      ~~~~~~~~~~~~~~~~~~~
tests/server/sws.c:777:8: note: Assuming 'got_exit_signal' is 0
  777 |     if(got_exit_signal)
      |        ^~~~~~~~~~~~~~~
tests/server/sws.c:777:5: note: Taking false branch
  777 |     if(got_exit_signal)
      |     ^
tests/server/sws.c:779:8: note: Assuming 'written' is > 0
  779 |     if(written > 0)
      |        ^~~~~~~~~~~
tests/server/sws.c:779:5: note: Taking true branch
  779 |     if(written > 0)
      |     ^
tests/server/sws.c:782:12: note: Assuming 'writeleft' is > 0
  782 |   } while((writeleft > 0) && ((error = errno) == EINTR));
      |            ^~~~~~~~~~~~~
tests/server/sws.c:782:11: note: Left side of '&&' is true
  782 |   } while((writeleft > 0) && ((error = errno) == EINTR));
      |           ^
tests/server/sws.c:782:40: note: An undefined value may be read from 'errno'
  782 |   } while((writeleft > 0) && ((error = errno) == EINTR));
      |                                        ^
8 warnings generated.
```
```
tests/libtest/lib1538.c:46:6: error: unmatched 'NOLINTEND' comment without a previous 'NOLINTBEGIN' comment [clang-tidy-nolint]
```
```
tests/unit/unit1300.c:63:10: error: Value stored to 'llist_size' during its initialization is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
   63 |   size_t llist_size = Curl_llist_count(&llist);
      |          ^~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~
tests/unit/unit1300.c:63:10: note: Value stored to 'llist_size' during its initialization is never read
   63 |   size_t llist_size = Curl_llist_count(&llist);
      |          ^~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~
```

Ref ba235ab
```
tests/unit/unit1620.c:61:34: error: Null pointer passed as 1st argument to string comparison function [clang-analyzer-unix.cstring.NullArg,-warnings-as-errors]
   61 |     fail_unless(!exp_username || strcmp(userstr, exp_username) == 0,
      |                                  ^
tests/unit/unit1620.c:63:34: error: Null pointer passed as 1st argument to string comparison function [clang-analyzer-unix.cstring.NullArg,-warnings-as-errors]
   63 |     fail_unless(!exp_password || strcmp(passwdstr, exp_password) == 0,
      |                                  ^
tests/unit/unit1620.c:65:33: error: Null pointer passed as 1st argument to string comparison function [clang-analyzer-unix.cstring.NullArg,-warnings-as-errors]
   65 |     fail_unless(!exp_options || strcmp(options, exp_options) == 0,
      |                                 ^
```
```
tests/unit/unit1663.c:69:29: error: Null pointer passed as 1st argument to string comparison function [clang-analyzer-unix.cstring.NullArg,-warnings-as-errors]
tests/unit/unit1663.c:71:31: error: Null pointer passed as 1st argument to string comparison function [clang-analyzer-unix.cstring.NullArg,-warnings-as-errors]
tests/unit/unit1663.c:73:30: error: Null pointer passed as 1st argument to string comparison function [clang-analyzer-unix.cstring.NullArg,-warnings-as-errors]
```
```
tests/unit/unit1661.c:103:16: error: Array access (from variable 'buffer') results in a null pointer dereference [clang-analyzer-core.NullDereference,-warnings-as-errors]
  103 |   fail_unless(!buffer[3], "Duplicated data should have been truncated");
      |                ^
```
```
tests/unit/unit3200.c:109:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
tests/unit/unit3200.c:113:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
tests/unit/unit3200.c:121:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
tests/unit/unit3200.c:125:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
tests/unit/unit3200.c:133:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
tests/unit/unit3200.c:142:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
tests/unit/unit3200.c:151:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
tests/unit/unit3200.c:160:9: error: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
```
```
/Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk -fPIC -fvisibility=hidden -MD -MT lib/CMakeFiles/libcurl_shared.dir/cf-socket.c.o -MF lib/CMakeFiles/libcurl_shared.dir/cf-socket.c.o.d -o lib/CMakeFiles/libcurl_shared.dir/cf-socket.c.o -c /Users/runner/work/curl/curl/lib/cf-socket.c
/Users/runner/work/curl/curl/lib/cf-socket.c:1831:8: error: The 1st argument to 'connect' is -1 but should be >= 0 [clang-analyzer-unix.StdCLibraryFunctions,-warnings-as-errors]
 1831 |   rc = connect(ctx->sock, &ctx->addr.curl_sa_addr,
      |        ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1887:6: note: Assuming field 'connected' is 0
 1887 |   if(cf->connected) {
      |      ^~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1887:3: note: Taking false branch
 1887 |   if(cf->connected) {
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1892:6: note: Assuming the condition is true
 1892 |   if(ctx->sock == CURL_SOCKET_BAD) {
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1892:3: note: Taking true branch
 1892 |   if(ctx->sock == CURL_SOCKET_BAD) {
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1893:14: note: Calling 'cf_socket_open'
 1893 |     result = cf_socket_open(cf, data);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1114:3: note: Loop condition is false.  Exiting loop
 1114 |   DEBUGASSERT(ctx->sock == CURL_SOCKET_BAD);
      |   ^
/Users/runner/work/curl/curl/lib/curl_setup_once.h:301:24: note: expanded from macro 'DEBUGASSERT'
  301 | #define DEBUGASSERT(x) do { } while(0)
      |                        ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1130:6: note: 'result' is 0
 1130 |   if(result)
      |      ^~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1130:3: note: Taking false branch
 1130 |   if(result)
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1133:12: note: Calling 'set_remote_ip'
 1133 |   result = set_remote_ip(cf, data);
      |            ^~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1090:7: note: Value assigned to field 'sock'
 1090 |   if(!Curl_addr2string(&ctx->addr.curl_sa_addr,
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1091 |                        (curl_socklen_t)ctx->addr.addrlen,
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1092 |                        ctx->ip.remote_ip, &ctx->ip.remote_port)) {
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1090:6: note: Assuming the condition is false
 1090 |   if(!Curl_addr2string(&ctx->addr.curl_sa_addr,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1091 |                        (curl_socklen_t)ctx->addr.addrlen,
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1092 |                        ctx->ip.remote_ip, &ctx->ip.remote_port)) {
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1090:3: note: Taking false branch
 1090 |   if(!Curl_addr2string(&ctx->addr.curl_sa_addr,
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1133:12: note: Returning from 'set_remote_ip'
 1133 |   result = set_remote_ip(cf, data);
      |            ^~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1134:6: note: 'result' is 0
 1134 |   if(result)
      |      ^~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1134:3: note: Taking false branch
 1134 |   if(result)
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1138:6: note: Assuming field 'family' is not equal to AF_INET6
 1138 |   if(ctx->addr.family == AF_INET6) {
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1138:3: note: Taking false branch
 1138 |   if(ctx->addr.family == AF_INET6) {
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1144:11: note: 'data' is non-null
 1144 |     infof(data, "  Trying %s:%d...", ctx->ip.remote_ip, ctx->ip.remote_port);
      |           ^
/Users/runner/work/curl/curl/lib/curl_trc.h:118:31: note: expanded from macro 'infof'
  118 |   do { if(Curl_trc_is_verbose(data)) \
      |                               ^~~~
/Users/runner/work/curl/curl/lib/curl_trc.h:195:15: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |               ^~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1144:5: note: Left side of '&&' is true
 1144 |     infof(data, "  Trying %s:%d...", ctx->ip.remote_ip, ctx->ip.remote_port);
      |     ^
/Users/runner/work/curl/curl/lib/curl_trc.h:118:11: note: expanded from macro 'infof'
  118 |   do { if(Curl_trc_is_verbose(data)) \
      |           ^
/Users/runner/work/curl/curl/lib/curl_trc.h:195:14: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |              ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1144:5: note: Assuming field 'verbose' is 0
 1144 |     infof(data, "  Trying %s:%d...", ctx->ip.remote_ip, ctx->ip.remote_port);
      |     ^
/Users/runner/work/curl/curl/lib/curl_trc.h:118:11: note: expanded from macro 'infof'
  118 |   do { if(Curl_trc_is_verbose(data)) \
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/curl_trc.h:195:24: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |                        ^~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1144:5: note: Left side of '&&' is false
 1144 |     infof(data, "  Trying %s:%d...", ctx->ip.remote_ip, ctx->ip.remote_port);
      |     ^
/Users/runner/work/curl/curl/lib/curl_trc.h:118:11: note: expanded from macro 'infof'
  118 |   do { if(Curl_trc_is_verbose(data)) \
      |           ^
/Users/runner/work/curl/curl/lib/curl_trc.h:195:44: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |                                            ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1144:5: note: Loop condition is false.  Exiting loop
 1144 |     infof(data, "  Trying %s:%d...", ctx->ip.remote_ip, ctx->ip.remote_port);
      |     ^
/Users/runner/work/curl/curl/lib/curl_trc.h:118:3: note: expanded from macro 'infof'
  118 |   do { if(Curl_trc_is_verbose(data)) \
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1147:13: note: Assuming field 'family' is not equal to AF_INET
 1147 |   is_tcp = (ctx->addr.family == AF_INET
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1147:13: note: Left side of '||' is false
/Users/runner/work/curl/curl/lib/cf-socket.c:1148:26: note: Field 'family' is not equal to AF_INET6
 1148 |             || ctx->addr.family == AF_INET6) &&
      |                          ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1148:46: note: Left side of '&&' is false
 1148 |             || ctx->addr.family == AF_INET6) &&
      |                                              ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1154:6: note: 'is_tcp' is false
 1154 |   if(is_tcp && data->set.tcp_nodelay)
      |      ^~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1154:13: note: Left side of '&&' is false
 1154 |   if(is_tcp && data->set.tcp_nodelay)
      |             ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1159:3: note: Loop condition is false.  Exiting loop
 1159 |   Curl_sndbuf_init(ctx->sock);
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.h:91:29: note: expanded from macro 'Curl_sndbuf_init'
   91 | #define Curl_sndbuf_init(y) Curl_nop_stmt
      |                             ^
/Users/runner/work/curl/curl/lib/curl_setup.h:874:23: note: expanded from macro 'Curl_nop_stmt'
  874 | #define Curl_nop_stmt do { } while(0)
      |                       ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1161:6: note: 'is_tcp' is false
 1161 |   if(is_tcp && data->set.tcp_keepalive)
      |      ^~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1161:13: note: Left side of '&&' is false
 1161 |   if(is_tcp && data->set.tcp_keepalive)
      |             ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1164:6: note: Assuming field 'fsockopt' is non-null
 1164 |   if(data->set.fsockopt) {
      |      ^~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1164:3: note: Taking true branch
 1164 |   if(data->set.fsockopt) {
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1172:8: note: Assuming 'error' is equal to CURL_SOCKOPT_ALREADY_CONNECTED
 1172 |     if(error == CURL_SOCKOPT_ALREADY_CONNECTED)
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1172:5: note: Taking true branch
 1172 |     if(error == CURL_SOCKOPT_ALREADY_CONNECTED)
      |     ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1182:16: note: Field 'family' is not equal to AF_INET
 1182 |   if(ctx->addr.family == AF_INET
      |                ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1182:6: note: Left side of '||' is false
 1182 |   if(ctx->addr.family == AF_INET
      |      ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1184:19: note: Field 'family' is not equal to AF_INET6
 1184 |      || ctx->addr.family == AF_INET6
      |                   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1182:3: note: Taking false branch
 1182 |   if(ctx->addr.family == AF_INET
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1204:6: note: Assuming 'error' is >= 0
 1204 |   if(error < 0) {
      |      ^~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1204:3: note: Taking false branch
 1204 |   if(error < 0) {
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1221:26: note: Assuming field 'socktype' is equal to SOCK_DGRAM
 1221 |   ctx->sock_connected = (ctx->addr.socktype != SOCK_DGRAM);
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1223:6: note: 'result' is 0
 1223 |   if(result) {
      |      ^~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1223:3: note: Taking false branch
 1223 |   if(result) {
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1229:11: note: 'isconnected' is true
 1229 |   else if(isconnected) {
      |           ^~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1229:8: note: Taking true branch
 1229 |   else if(isconnected) {
      |        ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1234:15: note: 'data' is non-null
 1234 |   CURL_TRC_CF(data, cf, "cf_socket_open() -> %d, fd=%" FMT_SOCKET_T,
      |               ^
/Users/runner/work/curl/curl/lib/curl_trc.h:124:38: note: expanded from macro 'CURL_TRC_CF'
  124 |   do { if(Curl_trc_cf_is_verbose(cf, data)) \
      |                                      ^~~~
/Users/runner/work/curl/curl/lib/curl_trc.h:199:34: note: expanded from macro 'Curl_trc_cf_is_verbose'
  199 |             (Curl_trc_is_verbose(data) && \
      |                                  ^~~~
/Users/runner/work/curl/curl/lib/curl_trc.h:195:15: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |               ^~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1234:3: note: Left side of '&&' is true
 1234 |   CURL_TRC_CF(data, cf, "cf_socket_open() -> %d, fd=%" FMT_SOCKET_T,
      |   ^
/Users/runner/work/curl/curl/lib/curl_trc.h:124:11: note: expanded from macro 'CURL_TRC_CF'
  124 |   do { if(Curl_trc_cf_is_verbose(cf, data)) \
      |           ^
/Users/runner/work/curl/curl/lib/curl_trc.h:199:14: note: expanded from macro 'Curl_trc_cf_is_verbose'
  199 |             (Curl_trc_is_verbose(data) && \
      |              ^
/Users/runner/work/curl/curl/lib/curl_trc.h:195:14: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |              ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1234:3: note: Assuming field 'verbose' is 0
 1234 |   CURL_TRC_CF(data, cf, "cf_socket_open() -> %d, fd=%" FMT_SOCKET_T,
      |   ^
/Users/runner/work/curl/curl/lib/curl_trc.h:124:11: note: expanded from macro 'CURL_TRC_CF'
  124 |   do { if(Curl_trc_cf_is_verbose(cf, data)) \
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/curl_trc.h:199:14: note: expanded from macro 'Curl_trc_cf_is_verbose'
  199 |             (Curl_trc_is_verbose(data) && \
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/curl_trc.h:195:24: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |                        ^~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1234:3: note: Left side of '&&' is false
 1234 |   CURL_TRC_CF(data, cf, "cf_socket_open() -> %d, fd=%" FMT_SOCKET_T,
      |   ^
/Users/runner/work/curl/curl/lib/curl_trc.h:124:11: note: expanded from macro 'CURL_TRC_CF'
  124 |   do { if(Curl_trc_cf_is_verbose(cf, data)) \
      |           ^
/Users/runner/work/curl/curl/lib/curl_trc.h:199:14: note: expanded from macro 'Curl_trc_cf_is_verbose'
  199 |             (Curl_trc_is_verbose(data) && \
      |              ^
/Users/runner/work/curl/curl/lib/curl_trc.h:195:44: note: expanded from macro 'Curl_trc_is_verbose'
  195 |             ((data) && (data)->set.verbose && \
      |                                            ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1234:3: note: Loop condition is false.  Exiting loop
 1234 |   CURL_TRC_CF(data, cf, "cf_socket_open() -> %d, fd=%" FMT_SOCKET_T,
      |   ^
/Users/runner/work/curl/curl/lib/curl_trc.h:124:3: note: expanded from macro 'CURL_TRC_CF'
  124 |   do { if(Curl_trc_cf_is_verbose(cf, data)) \
      |   ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1893:14: note: Returning from 'cf_socket_open'
 1893 |     result = cf_socket_open(cf, data);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1894:8: note: 'result' is 0
 1894 |     if(result) {
      |        ^~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1894:5: note: Taking false branch
 1894 |     if(result) {
      |     ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1899:8: note: Assuming field 'transport' is equal to TRNSPRT_QUIC
 1899 |     if(ctx->transport == TRNSPRT_QUIC) {
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1899:5: note: Taking true branch
 1899 |     if(ctx->transport == TRNSPRT_QUIC) {
      |     ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1900:16: note: Calling 'cf_udp_setup_quic'
 1900 |       result = cf_udp_setup_quic(cf, data);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/cf-socket.c:1829:3: note: Loop condition is false.  Exiting loop
 1829 |   DEBUGASSERT(ctx->sock != CURL_SOCKET_BAD);
      |   ^
/Users/runner/work/curl/curl/lib/curl_setup_once.h:301:24: note: expanded from macro 'DEBUGASSERT'
  301 | #define DEBUGASSERT(x) do { } while(0)
      |                        ^
/Users/runner/work/curl/curl/lib/cf-socket.c:1831:8: note: The 1st argument to 'connect' is -1 but should be >= 0
 1831 |   rc = connect(ctx->sock, &ctx->addr.curl_sa_addr,
      |        ^       ~~~~~~~~~
1 warning generated.
```
@vszakats vszakats changed the title tests: fix clang-tidy warnings cmake: fix clang-tidy builds to verify tests, fix fallouts Mar 18, 2025
@vszakats
Copy link
Member Author

vszakats commented Mar 18, 2025

This was frustrating. It's possible I made mistakes in this bugfix marathon. I appreciate reviews.

edit: thought it's over, but it isn't, more issues in CI.

```
/Users/runner/work/curl/curl/tests/libtest/lib568.c:82:3: error: The 1st argument to 'fstat' is -1 but should be >= 0 [clang-analyzer-unix.StdCLibraryFunctions,-warnings-as-errors]
   82 |   fstat(sdp, &file_info);
      |   ^     ~~~
```
https://github.com/curl/curl/actions/runs/13928244281/job/38978522794?pr=16756#step:14:617
@vszakats vszakats closed this in 9465327 Mar 24, 2025
@vszakats vszakats deleted the tidyenum branch March 24, 2025 09:17
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
- cmake: disable test bundles for clang-tidy builds.
  clang-tidy ignores #included .c sources, and incompatible with unity
  and bundles. It caused clang-tidy ignoring all test sources. It also
  means this is the first time tests sources are checked with
  clang-tidy. (autotools doesn't run it on tests.)

- cmake: update description for `CURL_TEST_BUNDLES` option.

- fix tests using special `CURLE_*` enums that were missing from
  `curl/curl.h`. Add them as reserved codes.

- fix about ~50 other issues detected by clang-tidy: unchecked results,
  NULL derefs, memory leaks, casts to enums, unused assigments,
  uninitialized `errno` uses, unchecked `open`, indent, and more.

- drop unnecessary casts (lib1533, lib3207).

- suppress a few impossible cases with detailed `NOLINT`s.

- lib/escape.c: drop `NOLINT` no longer necessary.
  Follow-up to 72abf7c curl#13862 (possibly)

- extend two existing `NOLINT` comments with details.

Follow-up to fabfa8e curl#15825

Closes curl#16756
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

1 participant