cmake: fix clang-tidy builds to verify tests, fix fallouts by vszakats · Pull Request #16756 · curl/curl

vszakats · 2025-03-17T23:45:04Z

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?

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

…PROTOCOL

…ROTOCOL

``` ../../../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

d485177

``` 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] ```

This reverts commit cb62b50.

``` /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 · 2025-03-18T16:45:11Z

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

- 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

vszakats added the build label Mar 17, 2025

github-actions bot added tests CI Continuous Integration labels Mar 17, 2025

This comment was marked as resolved.

Sign in to view

vszakats changed the title ~~libauthretry: silence clang-tidy warnings~~ tests: fix clang-tidy warnings Mar 18, 2025

github-actions bot added the libcurl API label Mar 18, 2025

vszakats force-pushed the tidyenum branch 2 times, most recently from 1e62535 to d0bef8e Compare March 18, 2025 03:25

vszakats force-pushed the tidyenum branch from 0598fb8 to f7ccf45 Compare March 18, 2025 12:48

vszakats added 20 commits March 18, 2025 14:20

reserve test enum values

f08003e

lib1301: fix to return TEST_ERR_FAILURE instead of CURLE_UNSUPPORTED_…

ff2a67f

…PROTOCOL

lib1501: replace literal with CURLE value

7938301

lib1559: fix to return TEST_ERR_FAILURE instead of CURLE_UNSUPPORTED_…

51ec9f9

…PROTOCOL

lib1911: fix to return TEST_ERR_FAILURE instead of CURLE_UNSUPPORTED_…

fb7d918

…PROTOCOL

lib1905: fix to return TEST_ERR_FAILURE instead of CURLE_UNSUPPORTED_…

35e0ced

…PROTOCOL

lib537: fix to return TEST_ERR_FAILURE instead of CURLE_UNSUPPORTED_P…

c31e94a

…ROTOCOL

lib1905.c

28fbd81

lib1911.c

9fc6700

lib1559.c

dcf21b8

test.h: replace 1 with CURLE_UNSUPPORTED_PROTOCOL

918b2aa

lib643.c: 1 -> TEST_ERR_FAILURE

5ecf60e

lib570.c: 1 -> TEST_ERR_FAILURE

94a2b39

lib1517.c: 1 -> TEST_ERR_FAILURE

124b856

lib518.c: 1 -> TEST_ERR_FAILURE

6a908bf

lib537.c: 1 -> TEST_ERR_MAJOR_BAD

0f3f3c2

lib540.c: 99 -> TEST_ERR_MAJOR_BAD

c42803d

lib3026.c: -1 -> TEST_ERR_MAJOR_BAD

7e040c3

lib3026.c: -1 -> TEST_ERR_MAJOR_BAD and type cleanup

8625058

lib3026.c: -1 -> TEST_ERR_MAJOR_BAD and type cleanup

824e8b6

vszakats added 2 commits March 18, 2025 15:26

test1477.pl: fixup OK

6313a0c

sigh

9354f14

vszakats force-pushed the tidyenum branch from 0bfe55d to 9354f14 Compare March 18, 2025 14:45

vszakats added 14 commits March 18, 2025 16:06

server: formatting

55b42f7

lib1538.c fixup fixup fixup

167d49e

``` tests/libtest/lib1538.c:46:6: error: unmatched 'NOLINTEND' comment without a previous 'NOLINTBEGIN' comment [clang-tidy-nolint] ```

unit1654: fix duplicate test ID (4) in error output

cbf9a85

unit1654: check results for the recently added 4 new tests

5585bb1

d485177

unit1661: avoid NULL deref

53ac243

``` 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"); | ^ ```

lib1559.c: fixup fixup fixup. fixup

0592969

Revert "cf-socket.c: try without NOLINT"

3473e3f

This reverts commit cb62b50.

vszakats changed the title ~~tests: fix clang-tidy warnings~~ cmake: fix clang-tidy builds to verify tests, fix fallouts Mar 18, 2025

vszakats added 7 commits March 18, 2025 17:52

lib572.c: check open() error

4f13ffc

lib579.c: fopen error check

8f3de23

lib579.c: fopen error check 2

e9f8fac

lib3207: avoid casts

f4e543b

lib1533: avoid casts

941a22d

lib3207: avoid casts fixup

0f552fd

vszakats closed this in 9465327 Mar 24, 2025

vszakats deleted the tidyenum branch March 24, 2025 09:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

cmake: fix clang-tidy builds to verify tests, fix fallouts#16756
vszakats wants to merge 85 commits intocurl:masterfrom
vszakats:tidyenum

vszakats commented Mar 17, 2025 •

edited

Loading

Uh oh!

This comment was marked as resolved.

vszakats commented Mar 18, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

vszakats commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

vszakats commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

vszakats commented Mar 17, 2025 •

edited

Loading

vszakats commented Mar 18, 2025 •

edited

Loading