On Wed, 6 Nov 2024 10:25:16 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:I've been experimenting with Zed and clangd recently. Currently it generates an enormous number of largely spurious errors and warnings on the passt code base. Mostly that's due to its default configurations not suiting us. This series adds some configuration that addresses a number of those warnings, though there remain many more for now. Some of the warnings also look reasonable, so I have a grab bag of fixes or workarounds for some of those two.This looks good to me. I tested build and functionality on Alpine x86, Debian armhf testing, Debian i686 testing, Debian amd64 testing, Fedora Rawhide x86 with this series plus: - [PATCH] fwd: Squash different-signedness comparison warning - [PATCH 0/8] Avoid running cppcheck on system headers on top. Other than single comments about "[PATCH 0/8] Avoid running cppcheck on system headers", the only issue this series adds is, with clang-tidy 16 (current version on Debian testing), a rain of: /home/sbrivio/passt/.clang-tidy:3:5: error: unexpected scalar - "clang-diagnostic-*,clang-analyzer-*,*,-modernize-*" ^ but it's fine, using clang-tidy 18 (on armhf) and clang-tidy 19 (everywhere else) fixes that. There are a bunch of pre-existing cppcheck and clang-tidy warnings that remain after this, and I plan to deal with them: 1. clang-tidy 19 on 32-bit architectures: -- /home/sbrivio/passt/tap.c:1087:16: error: comparison of integers of different signs: 'ssize_t' (aka 'int') and 'unsigned int' [clang-diagnostic-sign-compare,-warnings-as-errors] for (n = 0; n <= (ssize_t)TAP_BUF_BYTES - ETH_MAX_MTU; n += len) { ~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/sbrivio/passt/tcp.c:728:11: error: performing an implicit widening conversion to type 'uint64_t' (aka 'unsigned long long') of a multiplication performed in type 'unsigned long' [bugprone-implicit-widening-of-multiplication-result,-warnings-as-errors] if (v >= SNDBUF_BIG) ^ /home/sbrivio/passt/util.h:149:22: note: expanded from macro 'SNDBUF_BIG' #define SNDBUF_BIG (4UL * 1024 * 1024) ^ /home/sbrivio/passt/tcp.c:728:11: note: make conversion explicit to silence this warning if (v >= SNDBUF_BIG) ^ /home/sbrivio/passt/util.h:149:22: note: expanded from macro 'SNDBUF_BIG' #define SNDBUF_BIG (4UL * 1024 * 1024) ^~~~~~~~~~~~~~~~~ /home/sbrivio/passt/tcp.c:728:11: note: perform multiplication in a wider type if (v >= SNDBUF_BIG) ^ /home/sbrivio/passt/util.h:149:22: note: expanded from macro 'SNDBUF_BIG' #define SNDBUF_BIG (4UL * 1024 * 1024) ^~~~~~~~~~ /home/sbrivio/passt/tcp.c:730:15: error: performing an implicit widening conversion to type 'uint64_t' (aka 'unsigned long long') of a multiplication performed in type 'unsigned long' [bugprone-implicit-widening-of-multiplication-result,-warnings-as-errors] else if (v > SNDBUF_SMALL) ^ /home/sbrivio/passt/util.h:150:24: note: expanded from macro 'SNDBUF_SMALL' #define SNDBUF_SMALL (128UL * 1024) ^ /home/sbrivio/passt/tcp.c:730:15: note: make conversion explicit to silence this warning else if (v > SNDBUF_SMALL) ^ /home/sbrivio/passt/util.h:150:24: note: expanded from macro 'SNDBUF_SMALL' #define SNDBUF_SMALL (128UL * 1024) ^~~~~~~~~~~~ /home/sbrivio/passt/tcp.c:730:15: note: perform multiplication in a wider type else if (v > SNDBUF_SMALL) ^ /home/sbrivio/passt/util.h:150:24: note: expanded from macro 'SNDBUF_SMALL' #define SNDBUF_SMALL (128UL * 1024) ^~~~~ /home/sbrivio/passt/tcp.c:731:17: error: performing an implicit widening conversion to type 'uint64_t' (aka 'unsigned long long') of a multiplication performed in type 'unsigned long' [bugprone-implicit-widening-of-multiplication-result,-warnings-as-errors] v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2; ^ /home/sbrivio/passt/util.h:150:24: note: expanded from macro 'SNDBUF_SMALL' #define SNDBUF_SMALL (128UL * 1024) ^ /home/sbrivio/passt/tcp.c:731:17: note: make conversion explicit to silence this warning v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2; ^ /home/sbrivio/passt/util.h:150:24: note: expanded from macro 'SNDBUF_SMALL' #define SNDBUF_SMALL (128UL * 1024) ^~~~~~~~~~~~ /home/sbrivio/passt/tcp.c:731:17: note: perform multiplication in a wider type v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2; ^ /home/sbrivio/passt/util.h:150:24: note: expanded from macro 'SNDBUF_SMALL' #define SNDBUF_SMALL (128UL * 1024) ^~~~~ -- 2. cppcheck 2.16 on 32-bit only (!): -- dhcpv6.c:334:14: style: The comparison 'ia_type == 3' is always true. [knownConditionTrueFalse] if (ia_type == OPT_IA_NA) { ^ dhcpv6.c:306:12: note: 'ia_type' is assigned value '3' here. ia_type = OPT_IA_NA; ^ dhcpv6.c:334:14: note: The comparison 'ia_type == 3' is always true. if (ia_type == OPT_IA_NA) { ^ -- 3. clang-tidy 19.1.2 on Alpine x86: -- /home/sbrivio/passt/log.c:216:3: error: misleading indentation: statement is indented too deeply [readability-misleading-indentation,-warnings-as-errors] 216 | logfile_rotate_move(fd, now); | ^ /home/sbrivio/passt/log.c:207:2: note: did you mean this line to be inside this 'if' 207 | if (fcntl(fd, F_SETFL, O_RDWR /* Drop O_APPEND: explicit lseek() */)) | ^ /home/sbrivio/passt/passt.c:314:53: error: conditional operator with identical true and false expressions [bugprone-branch-clone,-warnings-as-errors] 314 | nfds = epoll_wait(c.epollfd, events, EPOLL_EVENTS, TIMER_INTERVAL); | ^ /home/sbrivio/passt/passt.c:60:29: note: expanded from macro 'TIMER_INTERVAL' 60 | #define TIMER_INTERVAL MIN(TIMER_INTERVAL_, FLOW_TIMER_INTERVAL) | ^ /home/sbrivio/passt/passt.c:59:30: note: expanded from macro 'TIMER_INTERVAL_' 59 | #define TIMER_INTERVAL_ MIN(TIMER_INTERVAL__, ICMP_TIMER_INTERVAL) | ^ /home/sbrivio/passt/passt.c:58:26: note: expanded from macro 'TIMER_INTERVAL__' 58 | #define TIMER_INTERVAL__ MIN(TCP_TIMER_INTERVAL, UDP_TIMER_INTERVAL) | ^ /home/sbrivio/passt/util.h:46:33: note: expanded from macro 'MIN' 46 | #define MIN(x, y) (((x) < (y)) ? (x) : (y)) | ^ /home/sbrivio/passt/tap.c:1139:38: error: 'socket' should use SOCK_CLOEXEC where possible [android-cloexec-socket,-warnings-as-errors] 1139 | int fd = socket(AF_UNIX, SOCK_STREAM, 0); | ^ | | SOCK_CLOEXEC /home/sbrivio/passt/tap.c:1158:51: error: 'socket' should use SOCK_CLOEXEC where possible [android-cloexec-socket,-warnings-as-errors] 1158 | ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); | ^ | | SOCK_CLOEXEC /home/sbrivio/passt/tcp.c:1414:44: error: 'socket' should use SOCK_CLOEXEC where possible [android-cloexec-socket,-warnings-as-errors] 1414 | s = socket(af, SOCK_STREAM | SOCK_NONBLOCK, IPPROTO_TCP); | ^ | | SOCK_CLOEXEC /home/sbrivio/passt/util.c:186:38: error: 'socket' should use SOCK_CLOEXEC where possible [android-cloexec-socket,-warnings-as-errors] 186 | if ((s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0) { | ^ | | SOCK_CLOEXEC -- 4. cppcheck 2.14.2 on Alpine x86: -- dhcpv6.c:431:32: style: Variable 'client_id' can be declared as pointer to const [constVariablePointer] struct opt_hdr *ia, *bad_ia, *client_id; ^ util.h:168:0: information: Unmatched suppression: funcArgNamesDifferent [unmatchedSuppression] int close_range(unsigned int first, unsigned int last, int flags) { ^ -- I'll apply everything in a bit, minus 2/8 to 4/8 of "[PATCH 0/8] Avoid running cppcheck on system headers". -- Stefano