[CORE] [Clang]Cleanup clang-analyzer warnings #46259

smuzaffar · 2024-10-04T12:50:18Z

This PR fixes clang-analyzer warnings

cmsbuild · 2024-10-04T12:50:41Z

cms-bot internal usage

cmsbuild · 2024-10-04T12:54:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46259/42082

cmsbuild · 2024-10-04T12:54:29Z

A new Pull Request was created by @smuzaffar for master.

It involves the following packages:

Utilities/StaticAnalyzers (core)
Utilities/StorageFactory (core)

@Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please review it and eventually sign? Thanks.
@wddgit this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

makortel · 2024-10-04T14:35:30Z

FYI @gartung for the static analyzer (changes look good to me)

gartung · 2024-10-04T16:03:52Z

Looks OK to me. The asserts should cause the checker to exit with error, but the checker build will continue.

Utilities/StorageFactory/src/LocalFileSystem.cc

makortel · 2024-10-04T17:01:30Z

Utilities/StorageFactory/src/LocalFileSystem.cc

@@ -361,6 +361,7 @@ LocalFileSystem::FSInfo *LocalFileSystem::findMount(const char *path,

    prev_paths.push_back(path);
    LocalFileSystem::FSInfo *new_best = findMount(fullpath, &sfs2, &s2, prev_paths);
+    free(fullpath);


How about using a unique_ptr for the fullpath, e.g.

std::unique_ptr<char, decltype(std::free)> fullpath{realpath(best->origin, nullptr), std::free}; if (!fullpath) fullpath.reset(strdup(best->origin));

removing the free(fullpath) calls and replacing the accesses with fullpath.get()?

In principle the code could be updated to use std::filesystem, but that sounds a lot more work that would have to be done with care.

@makortel , I have tried std::unique_ptr and after replacing fullpath tofullpath.get() and few other places (lstat, statfs, findMount need const char*) I got build error

In file included from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:36, from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/memory:75, from /cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_2_X_2024-10-06-2300/src/FWCore/MessageLogger/interface/MessageLogger.h:23, from src/Utilities/StorageFactory/src/LocalFileSystem.cc:4: /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/tuple: In instantiation of 'struct std::_Head_base<1, void(void*) noexcept, false>': /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/tuple:416:12: required from 'struct std::_Tuple_impl<1, void(void*) noexcept>' /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/tuple:258:12: required from 'struct std::_Tuple_impl<0, char*, void(void*) noexcept>' /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/tuple:981:11: required from 'class std::tuple<char*, void(void*) noexcept>' /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:224:27: required from 'class std::__uniq_ptr_impl<char, void(void*) noexcept>' /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:255:12: required from 'struct std::__uniq_ptr_data<char, void(void*) noexcept, false, false>' /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:275:33: required from 'class std::unique_ptr<char, void(void*) noexcept>' src/Utilities/StorageFactory/src/LocalFileSystem.cc:341:48: required from here /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02858/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/tuple:238:13: error: data member 'std::_Head_base<1, void(void*) noexcept, false>::_M_head_impl' invalidly declared function type 238 | _Head _M_head_impl; | ^~~~~~~~~~~~ src/Utilities/StorageFactory/src/LocalFileSystem.cc: In member function 'edm::storage::LocalFileSystem::FSInfo* edm::storage::LocalFileSystem::findMount(const char*, statfs*, stat*, std::vector<std::__cxx11::basic_string<char> >&) const': src/Utilities/StorageFactory/src/LocalFileSystem.cc:341:100: error: no matching function for call to 'std::unique_ptr<char, void(void*) noexcept>::unique_ptr(<brace-enclosed initializer list>)' 341 | std::unique_ptr<char, decltype(std::free)> full_path{realpath(best->origin, nullptr), std::free};

If I change

std::unique_ptr<char, decltype(std::free)> fullpath{realpath(best->origin, nullptr), std::free};

to

std::unique_ptr<char, decltype(std::free) *> fullpath{realpath(best->origin, nullptr), std::free};

then build error go away. So is std::unique_ptr<char, decltype(std::free) *> the correct thing touse in this case?

So is std::unique_ptr<char, decltype(std::free) *> the correct thing touse in this case?

Ah, yes, it is the correct type. Sorry for omitting the *.

cmsbuild · 2024-10-07T07:32:05Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46259/42100

cmsbuild · 2024-10-07T07:32:23Z

Pull request #46259 was updated. @Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please check and sign again.

smuzaffar · 2024-10-07T07:33:03Z

please test

cmsbuild · 2024-10-07T10:42:11Z

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-965b2b/42005/summary.html
COMMIT: 3f2c77c
CMSSW: CMSSW_14_2_X_2024-10-06-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/46259/42005/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 2 lines to the logs
Reco comparison results: 14 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3331336
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3331308
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

Co-authored-by: Matti Kortelainen <[email protected]>

cmsbuild · 2024-10-07T14:51:21Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46259/42112

cmsbuild · 2024-10-07T14:51:42Z

Pull request #46259 was updated. @Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please check and sign again.

smuzaffar · 2024-10-07T15:07:12Z

please test

cmsbuild · 2024-10-07T19:34:35Z

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-965b2b/42027/summary.html
COMMIT: 49c17d9
CMSSW: CMSSW_14_2_X_2024-10-07-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/46259/42027/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 2 lines from the logs
Reco comparison results: 105 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3331336
DQMHistoTests: Total failures: 3234
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3328082
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

makortel · 2024-10-07T19:51:01Z

Comparison differences are related to #39803

makortel · 2024-10-07T19:52:02Z

+core

cmsbuild · 2024-10-07T19:52:23Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @antoniovilela, @sextonkennedy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

mandrenguyen · 2024-10-08T06:49:41Z

+1

[CORE] [Clang]Cleanup clang-analyzer warnings

401976a

cmsbuild added this to the CMSSW_14_2_X milestone Oct 4, 2024

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Oct 4, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 4, 2024

makortel reviewed Oct 4, 2024

View reviewed changes

LocalFileSystem: use unique_ptr

3f2c77c

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 7, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 7, 2024

cmsbuild added tests-started and removed tests-pending labels Oct 7, 2024

cmsbuild added tests-approved and removed tests-started labels Oct 7, 2024

Update Utilities/StorageFactory/src/LocalFileSystem.cc

49c17d9

Co-authored-by: Matti Kortelainen <[email protected]>

cmsbuild removed the tests-approved label Oct 7, 2024

cmsbuild added tests-pending code-checks-pending and removed code-checks-approved labels Oct 7, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 7, 2024

cmsbuild added tests-started and removed tests-pending labels Oct 7, 2024

cmsbuild added tests-approved and removed tests-started labels Oct 7, 2024

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Oct 7, 2024

cmsbuild added orp-approved and removed orp-pending labels Oct 8, 2024

cmsbuild merged commit aaf9cc0 into cms-sw:master Oct 8, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CORE] [Clang]Cleanup clang-analyzer warnings #46259

[CORE] [Clang]Cleanup clang-analyzer warnings #46259

smuzaffar commented Oct 4, 2024

cmsbuild commented Oct 4, 2024 •

edited

Loading

cmsbuild commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

makortel commented Oct 4, 2024

gartung commented Oct 4, 2024

makortel Oct 4, 2024

smuzaffar Oct 7, 2024

makortel Oct 7, 2024

cmsbuild commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

smuzaffar commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

smuzaffar commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

makortel commented Oct 7, 2024

makortel commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

mandrenguyen commented Oct 8, 2024

[CORE] [Clang]Cleanup clang-analyzer warnings #46259

[CORE] [Clang]Cleanup clang-analyzer warnings #46259

Conversation

smuzaffar commented Oct 4, 2024

cmsbuild commented Oct 4, 2024 • edited Loading

cmsbuild commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

makortel commented Oct 4, 2024

gartung commented Oct 4, 2024

makortel Oct 4, 2024

Choose a reason for hiding this comment

smuzaffar Oct 7, 2024

Choose a reason for hiding this comment

makortel Oct 7, 2024

Choose a reason for hiding this comment

cmsbuild commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

smuzaffar commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

Comparison Summary

cmsbuild commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

smuzaffar commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

Comparison Summary

makortel commented Oct 7, 2024

makortel commented Oct 7, 2024

cmsbuild commented Oct 7, 2024

mandrenguyen commented Oct 8, 2024

cmsbuild commented Oct 4, 2024 •

edited

Loading