Make it easier to disable process executor

[sodero]

By adding a DISALLOW_PROCESS_EXECUTOR build option it's possible to conveniently disable the usage of fork() on non-Window platforms that either don't have fork(), or have an incomplete or inefficient implementation that is to be considered a last resort only. In the same commit the DISALLOW_THREAD_EXECUTOR is used for conditionally compiling the thread executor so that both executors are treated in the same way.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[firewave]

Thanks for your contribution.

Why do you need to manually disable this?

The idea of not being able to disable the process code was that it is what should always be present since it is more stable (i.e. only the subprocess will exit on a crash/assertion). So being able to disable thread code makes sure it is not accidentally used.

Aligning it makes sense though as if we only need/want to use the thread code (e.g. for performance reasons) there is no need to include the process code at all.

On a side note - adding process execution for Windows is tracked via https://trac.cppcheck.net/ticket/12464. I have not looked into it yet since I am still working on reworking the execution code so it can be shared which in turn is currently blocked by the suppressions not working properly (i.e. being able to merge #3090 - latest work related to it is #7346).

Also some observations outside of the scope of this PR (needs tickets to be filed - and should probably be held off until https://trac.cppcheck.net/ticket/14102 has been resolved):

• we are using WIN32 instead of _WIN32 in several preprocessor checks (WIN32 is only manually defined in our Visual Studio projects)
• processexecutor.cpp should check HAS_THREADING_MODEL_FORK
• the preprocessor checks in config.h and processexecutor.cpp differ
• ProcessExecutor is still available even if the code is not compiled in
• the threadexecutor.cpp code should not be compiled if it is disabled

[sodero]

Thanks for your quick response!

The reason for needing to manually disable the process executor is that I'm building Cppcheck for platforms without a proper fork (just vfork). These OS:es also lack proper memory protection so the stability benefits that you normally get from using a process rather than a thread don't exist. Rather than testing for these OS:es, like it's done with WIN32 today, I opted for the proposed solution, especially since what I'm doing is a very niche thing and would just clutter your code. Since it's a question of disabling, not enabling, I don't think the risk of accidentally using the less robust thread configuration is very high.

[firewave]

Thanks for the insight.

The reason for needing to manually disable the process executor is that I'm building Cppcheck for platforms without a proper fork (just vfork).

So does this mean it just behaves differently or does it not compile at all?

Since the preprocessor checks are still very light I would like to avoid another list of random arch checks (heck, this already got out of sync) as we have in other places and do a proper detection (at least in terms of CMake) to address the latter.

[sodero]

Unfortunately it doesn't build at all and proper detection isn't possible.

[firewave]

Unfortunately it doesn't build at all and proper detection isn't possible.

In CMake it can be detected via https://cmake.org/cmake/help/latest/module/CheckFunctionExists.html. But as I said it is not in the scope of this PR.

[sodero]

Unfortunately it doesn't build at all and proper detection isn't possible.

In CMake it can be detected via https://cmake.org/cmake/help/latest/module/CheckFunctionExists.html. But as I said it is not in the scope of this PR.

Yes, but CMake isn't available either. I'm far out in the dessert :)

[firewave]

Yes, but CMake isn't available either. I'm far out in the dessert :)

I assumed so. In that case the flag should be enough. And we should probably not add to the preprocessor in source and possibly add it to the Makefile instead. In that case it would be more in line with CMake.

[firewave]

• we are using WIN32 instead of _WIN32 in several preprocessor checks (WIN32 is only manually defined in our Visual Studio projects)

I filed https://trac.cppcheck.net/ticket/14198 about this.

[firewave]

I adjusted the existing preprocessor checks. Please rebase after #8010 has been merged.

Sorry it has taken so long.

[firewave]

Please also add an entry to releasenotes.txt.

After my changes DISALLOW_ is a bit of a misnomer - we should probably align it with the other options but we can do that later.

[sodero]

Sorry it has taken so long.

No problem, thanks for accepting patches for niche use cases.

[firewave]

Please also add a test for testprocessexecutor.cpp in CI-unixish.yml (see #8010).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sodero]

Done. I think.