From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH 00/12] Minor fixups for or inspired by clangd and related tools
Date: Thu, 7 Nov 2024 07:47:20 +1100 [thread overview]
Message-ID: <ZyvV2O7scHq7p-9L@zatzit> (raw)
In-Reply-To: <20241106201329.67440249@elisabeth>
[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]
On Wed, Nov 06, 2024 at 08:13:29PM +0100, Stefano Brivio wrote:
> On Wed, 6 Nov 2024 10:25:16 +1100
> David Gibson <david@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.
I thought this might just because I was mixing a string with a list of
options with a json/yaml list of options. Alas, no, I think
clang-tidy 16 only accepts a big string here rather than a yaml list.
Doing it as a string would prevent the interspersing of the
explanatory comments, so I don't think it's worth it.
> There are a bunch of pre-existing cppcheck and clang-tidy warnings that
> remain after this, and I plan to deal with them:
Ok. Do you need anything from me?
[snip]
> 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) {
> ^
Weird, looks like another false positive, maybe with the same cause as
that other knownConditionTrueFalse thing we hit.
> --
>
> 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);
> | ^
I think my FALLOC_FL_COLLAPSE_RANGE change should fix this as a side
effect - the odd indentation is because of the #ifdef cutting off part
ofthe if.
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-11-06 20:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-05 23:25 [PATCH 00/12] Minor fixups for or inspired by clangd and related tools David Gibson
2024-11-05 23:25 ` [PATCH 01/12] clang: Add .clang-format file David Gibson
2024-11-05 23:25 ` [PATCH 02/12] Makefile: Simplify exclusion of qrap from static checks David Gibson
2024-11-05 23:25 ` [PATCH 03/12] clang: Move clang-tidy configuration from Makefile to .clang-tidy David Gibson
2024-11-05 23:25 ` [PATCH 04/12] arch: Avoid explicit access to 'environ' David Gibson
2024-11-05 23:25 ` [PATCH 05/12] flow: Correct type of flowside_at_sidx() David Gibson
2024-11-05 23:25 ` [PATCH 06/12] netlink: RTA_PAYLOAD() returns int, not size_t David Gibson
2024-11-05 23:25 ` [PATCH 07/12] Makefile: Move NETNS_RUN_DIR definition to C code David Gibson
2024-11-05 23:25 ` [PATCH 08/12] seccomp: Simplify handling of AUDIT_ARCH David Gibson
2024-11-05 23:25 ` [PATCH 09/12] Makefile: Use -DARCH for qrap only David Gibson
2024-11-05 23:25 ` [PATCH 10/12] Makefile: Don't attempt to auto-detect stack size David Gibson
2024-11-05 23:25 ` [PATCH 11/12] clang: Add rudimentary clangd configuration David Gibson
2024-11-05 23:25 ` [PATCH 12/12] util: Remove unused ffsl() function David Gibson
2024-11-06 19:13 ` [PATCH 00/12] Minor fixups for or inspired by clangd and related tools Stefano Brivio
2024-11-06 20:47 ` David Gibson [this message]
2024-11-07 7:03 ` Stefano Brivio
2024-11-07 14:55 ` Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZyvV2O7scHq7p-9L@zatzit \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).