Fix use-after-move in L1MuDTTrackFinder, and some additional modernization #40411
Conversation
Also L1MuDTTrackFinder.h::reconstruct() function altogether as it was unused
Nothing depended on it being static
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40411/33538
|
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages:
@epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild, please test |
|
|
||
| std::vector<L1MuDTTrackCand>& getcache0() { return _cache0; } | ||
|
|
||
| std::vector<L1MuRegionalCand>& getcache() { return _cache; } | ||
|
|
||
| private: | ||
| /// run Track Finder and store candidates in cache | ||
| virtual void reconstruct(const edm::Event& e, const edm::EventSetup& c) { |
There was a problem hiding this comment.
This function appeared to be unused so I removed it.
| if (m_config->Debug(2)) | ||
| cout << "running " << (*it_sp).second->id() << endl; | ||
| if ((*it_sp).second) | ||
| (*it_sp).second->run(bx, e, c); | ||
| if (m_config->Debug(2) && (*it_sp).second) | ||
| (*it_sp).second->print(); |
There was a problem hiding this comment.
This pattern of
if (debug)
// access ptr
if (ptr)
// access ptr
if (debug && ptr)
// access ptrseems to repeat in this file many times. Given how the vectors and the map are filled, there should not be any nullptr entries, and therefore the if (ptr) check seems redundant. Also the nullness is not checked in the first if, so the code would (likely) crash anyway if, for any reason, the ptr would be null. Could this pattern be replaced with just
if (debug)
// access ptr
// access ptr
if (debug)
// access ptr?
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0199bd/29776/summary.html Comparison SummarySummary:
|
|
+l1 |
cmsbuild
added
fully-signed
l1-approved
and removed
l1-pending
pending-signatures
labels
|
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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This PR fixes a use-after-move of
edm::ConsumesCollectorinL1MuDTTrackFinder::setup()that was reported by the static analyzer. While doing that I made some additional modernization to the codestd::unique_ptrvirtualfunction specifiers (nothing inherits from the affected classes)config()a non-staticmember function (nothing required it to bestatic)forloopPR validation:
Code compiles