Update cppcheck and astyle versions

[matt335672]

Bumped astyle and cppcheck to latest versions.

There's an issue with cppcheck which I'd appreciate a comment on. Version 2.13.0 was taking around 20 minutes or so to run. With 2.14.0, I get warnings like this with the same sources:-

common/base64.c:0:0: information: Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches. [normalCheckLevelMaxBranches]

^
common/file.c:0:0: information: Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches. [normalCheckLevelMaxBranches]

^
common/list.c:0:0: information: Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches. [normalCheckLevelMaxBranches]

^
common/log.c:0:0: information: Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches. [normalCheckLevelMaxBranches]
...

If I use --check-level=exhaustive with 2.14.0, I get check times of around an hour:-

https://github.com/matt335672/xrdp/actions/runs/8781923998/job/24094941025

However, if I just disable the warnings and go with --check-level=normal, I'm getting run times of about 4 minutes, 30 seconds:-

https://github.com/matt335672/xrdp/actions/runs/8782777220/job/24097568007

Neither option seems as good to me, but I've gone for the shorter one. A longer run can still be requested manually.

Any thoughts appreciated.

[firewave]

It is a bit of a mess right now and discussions are ongoing about this.

--check-level=normal disables a whole part of ValueFlow which saw a massive performance regression in 2.14. So going for --check-level=exhaustive will seriously increase your run-time. Also the threshold for normalCheckLevelMaxBranches is currently not configurable and determined inconsistently and exceeded quite easily (https://trac.cppcheck.net/ticket/12508).

So suppressing the messages for now and leaving the --check-level=normal should be the way to go.

[matt335672]

Thanks for the update @firewave. That's an interesting link.

[firewave]

Also please CC me on any future Cppcheck-related things.

[matt335672]

cppcheck version updated to 2.14.1

Check times for both normal and extensive remain unaltered from 2.14.0

@firewave - I'm unsure what the best thing to do here is. Is it likely in later versions of cppcheck that we'll regain more rational behaviour without specifying a check_level? Or should we proceed with a check_level of normal for CI purposes, and keep an option of running an extensive check before releases?

[firewave]

I totally missed the question. I will provide a full reply later on. But the gist of it to use the highest level of checking.

Also I have been doing some profiling recently and provide some improvements so the next version could be faster out of the box (we again also hit some performance regressions - so mileage may vary). I am also in the midst of writing a preliminary tuning guide so that should also give you some more insights.

[firewave]

But the gist of it to use the highest level of checking.

I did not fully remember the post and did not read it again. So I missed the massive increase in runtime. You should keep it at normal and suppress the warning to keep the length of the job reasonable.

[matt335672]

Further investigations with astyle versions up to 3.5.1 revealed this problem:-

https://sourceforge.net/p/astyle/bugs/572/

This isn't good for us, as expressions like width * height become width *height. So I've left the astyle version at 3.4.14

@metalefty - are you OK to merge this?

[firewave]

cppcheck: Failed to load library configuration file 'std.cfg'. File not found
Failed to load std.cfg. Your Cppcheck installation is broken, please re-install. The Cppcheck binary was compiled without FILESDIR set. Either the std.cfg should be available in /home/runner/cppcheck.local/2.15.0/bin/cfg or the FILESDIR should be configured.

compiled without FILESDIR - the script is clearly passing the define. I have no idea why this fails. I tested it locally and it is working fine.

[firewave]

g++ -Ilib -isystem externals -isystem externals/picojson -isystem externals/simplecpp -isystem externals/tinyxml2 -DHAVE_BOOST -std=gnu++0x -pipe -c -o build/check.o build/check.cpp

So this is caused by CPPFLAGS=-DHAVE_BOOST.

[firewave]

I filed https://trac.cppcheck.net/ticket/13066. Will backport the fix to 2.15 but as we seem to have other issues the 2.15.1 patch release might take a day or two.

FYI I did a lot of performance improvements for 2.15 (more coming for 2.16). I hope it helps with the runtime so we add the remaining missing includes soon.

[matt335672]

Hi @firewave

I was just looking at this!

I'd just removed the -DHAVE_BOOST myself for 2.15.0. Is this a reasonable thing to do?

[firewave]

I'd just removed the -DHAVE_BOOST myself for 2.15.0. Is this a reasonable thing to do?

Not if you do not want to lose quite some performance...

And writing that the build already finished in 3 minutes. Which seems waaaaaaay too fast as it used to be almost 18 minutes.

So removing Boost for now is fine but I guess I have to have a look afterwards (when I find the time) why it is so damn fast (not that I am complaining).

[matt335672]

You're the expert - I'll wait to hear from you before I make any more changes in this area.

[firewave]

No need for further changes. This can be merged.

I will keep you in the loop when Boost can be enabled again.

[matt335672]

@metalefty - are you happy to merge this?

[metalefty]

Yes, let's go.