-
Notifications
You must be signed in to change notification settings - Fork 4.4k
[ML] [Clang]Cleanup clang-analyzer warnings #46318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46318/42147 |
A new Pull Request was created by @smuzaffar for master. It involves the following packages:
@cmsbuild, @valsdav, @y19y19 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
store = nullptr; | ||
assert(words); | ||
store = new Word_t[words]; | ||
std::memcpy(store, orig.store, words * sizeof(Word_t)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that bits_ > 0
and in that case bitsToWords(bits_)
should always return words>0
store = nullptr; | ||
assert(words); | ||
store = new Word_t[words]; | ||
std::memset(store, 0, sizeof(Word_t) * words); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that bits_ > 0
and in that case bitsToWords(bits_)
should always return words>0
please test |
-1 Failed Tests: UnitTests RelVals RelVals-INPUT AddOn Unit TestsI found 3 errors in the following unit tests: ---> test testPhysicsToolsMVAComputer had ERRORS ---> test runtestTqafTopJetCombination had ERRORS ---> test runtestTqafTopEventSelection had ERRORS RelVals
Expand to see more relval errors ...RelVals-INPUT
Expand to see more relval errors ...AddOn Tests
Expand to see more addon errors ... |
@cms-sw/ml-l2 , Looks like [VarProcessor::~VarProcessor()[(https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/MVAComputer/src/VarProcessor.cc#L42-L45) is explicitly calling Do you agree we can remove the following from the
|
@cms-sw/ml-l2 , what do you think about #46318 (comment) ? |
please test I have cleaned up |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46318/42179 |
+1 Size: This PR adds an extra 20KB to repository Comparison SummarySummary:
|
@cms-sw/ml-l2 , can youplease review this PR? |
Milestone for this pull request has been moved to CMSSW_15_0_X. Please open a backport if it should also go in to CMSSW_14_2_X. |
Milestone for this pull request has been moved to CMSSW_15_1_X. Please open a backport if it should also go in to CMSSW_15_0_X. |
This PR fixes clang-analyzer warnings