public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
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 --]

  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).