Re: [PATCH 00/12] Minor fixups for or inspired by clangd and related tools

7 Nov 2024


      On Thu, 7 Nov 2024 07:47:20 +1100
David Gibson  wrote:
...
On Wed, Nov 06, 2024 at 08:13:29PM +0100, Stefano Brivio wrote:
...
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.
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?
No, no, thanks, I think I already figured out most of those.
...
[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.
It could be a similar cause, but the fact that it only happens on armhf
and i686 makes me think it's not really the same.
...
...
--
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.
Oops, sorry, this actually goes away with the second series.

-- 
Stefano