From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=M56PDHmo; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 751315A004C for ; Wed, 06 Nov 2024 20:13:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730920416; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=PAM8accs5wbkIBO4IFEdqbPQT7brcBZSVbeehBwmocc=; b=M56PDHmoCSnTPq7oEH5f4Nj/fsDII1GN3e7JpRNmpqNE5omV4MdqLkNS/lhwbbT2qyHQ1m myAmLl+3qbGS4PG/KDjpR1q8CnivgBUn9YEqPXxcy8gE52rVNhUTyYIaY11ieiImBzQNRG d+cfKKO/4K3YY4JcpUVOFUDdx4wHsgs= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-131-Q3MHAQ-XMlCmErsItHEktA-1; Wed, 06 Nov 2024 14:13:34 -0500 X-MC-Unique: Q3MHAQ-XMlCmErsItHEktA-1 X-Mimecast-MFC-AGG-ID: Q3MHAQ-XMlCmErsItHEktA Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-431673032e6so947965e9.0 for ; Wed, 06 Nov 2024 11:13:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730920413; x=1731525213; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=PAM8accs5wbkIBO4IFEdqbPQT7brcBZSVbeehBwmocc=; b=Q9kgvoiRCyrHz2oF6gp2Bx7gbT6WSyuKKVFcDqINSYRMB2cIZeliONIr78skcCh6D3 ivj5ZskBMEZKpOXDlHDGOhHOoXdIfyl2QPmMJHHt2Eno1+9NaV38o7TPPn0GJTBqJGKL tzPVIoLNi3Prex+JSANbj2hjOE6/lRpLgpXAeo3vgWES83gjnhLjM0/Ax6fwm/l3uhtP RbDyP+VyhV2YfntA27pONg4TUZ+bRitz5QkQBv6Z7AuC/S7LeVHy9kb2F1vr0zmuf5BH /NgATuvv6N5sh+w/AFhiFVYzJLMp8T9F7KcQsIKM5PWz5e8Vq3W0XrxfyyQx0zHTkStA fl+Q== X-Gm-Message-State: AOJu0Yy/s5DXYZc8HubBzlcDFUU52RKgxcJvvVo0es8aanqwRoA202mq Ic5f0WEFl0EwxE95u4BgnWBmEhABdrq1CO0PrrLhTD3+fZiJ1R/yHufRwJWXs838f87u/pz9tNm SpRK8+18tEXpmQzI8QakzrQQic8To1h3OhltgGdTcw8vPOWdyxc0CkM0KHg== X-Received: by 2002:a05:600c:3547:b0:42c:b16e:7a22 with SMTP id 5b1f17b1804b1-431bb9855c0mr275547235e9.12.1730920412786; Wed, 06 Nov 2024 11:13:32 -0800 (PST) X-Google-Smtp-Source: AGHT+IFPcC84Tu+qhXTy9e2d4UZf5QkIqLnCf7P631t7/NwIyrThao0PcItbjf5PgxyzrsE+a7OPeQ== X-Received: by 2002:a05:600c:3547:b0:42c:b16e:7a22 with SMTP id 5b1f17b1804b1-431bb9855c0mr275547075e9.12.1730920412330; Wed, 06 Nov 2024 11:13:32 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-432aa6b2c32sm33368455e9.10.2024.11.06.11.13.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Nov 2024 11:13:31 -0800 (PST) Date: Wed, 6 Nov 2024 20:13:29 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 00/12] Minor fixups for or inspired by clangd and related tools Message-ID: <20241106201329.67440249@elisabeth> In-Reply-To: <20241105232528.1408144-1-david@gibson.dropbear.id.au> References: <20241105232528.1408144-1-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: NDrbe863SxeW3TcBSuV__5r-9QsOwVGP0-aV8MYVAQg_1730920414 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: LH4EYW6B6EDHGCX7XOCE4GLL6UNVQ6C5 X-Message-ID-Hash: LH4EYW6B6EDHGCX7XOCE4GLL6UNVQ6C5 X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, 6 Nov 2024 10:25:16 +1100 David Gibson 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