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