Issues found by PVS-Studio Static Analyzer

Article/Code review: CMake: the Case when the Project's Quality is Unforgivable.

changed the description

Thanks for the review @SvyatoslavRazmyslov!

Being a static analysis nerd, I was looking forward to a PVS-Studio analysis for a long time.

It is worth pointing out that many of the findings are actually the in 3rd-party libraries libuv, libcurl, rhash, and libarchive. Especially the last one seems to contribute many issues. You might want to analyze this one next?

The remaining issues come from CTest, CPack, and the Visual Studio specific code. I am surprised the the core of CMake is actually pretty clean.

added triage:mr-welcome label

Thanks! I'd welcome MRs fixing any of the problems not in the third-party code.

Though we should probably forward the third-party code issues up to their respective upstreams.

libuv PR: https://github.com/libuv/libuv/pull/2434

The linked report claims a memory leak in cmCTestMultiProcessHandler::StartTestProcess but there is no leak. In the case that StartTest returns true then ownership of the cmCTestRunTest instance has been moved to another structure. When the test finishes FinishTestProcess will be called later to delete it. Some refactoring could be done to clarify this and help static analysis tools understand it.

mentioned in merge request !3715 (merged)

!3715 (merged) fixes the issues in CMake/CTest/CPack proper. For now we do not fix vendored third-party sources which should be fixed upstream.