Fixed array overflow by ignoring the out-of-bounds value ... #9527
Conversation
…n/EcalDigis. Ignoring such values is ways better than modifying unrelated variables on the stack.
|
A new Pull Request was created by @dmitrijus (Dmitrijus) for CMSSW_7_5_X. Fixed array overflow by ignoring the out-of-bounds value ... It involves the following packages: Validation/EcalDigis @cmsbuild, @danduggan, @deguio can you please review it and eventually sign? Thanks. |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
+1 |
Fixed array overflow by ignoring the out-of-bounds value ...
| } else { | ||
| // FindFixBin might return an overflow bin (outside tpEtCount range). | ||
| // To prevent a memory overflow / segfault, these values are ignored. | ||
| std::cout << "EcalSelectiveReadoutValidation: Invalid iE value: " << iE << std::endl; |
There was a problem hiding this comment.
I thought, we shouldn't use std::cout. Shouldn't this be converted to message logger?
I see this is quite popular issue:
Begin processing the 88th record. Run 1, Event 7388, LumiSection 74 at 22-Jun-2015 15:37:19.177 CEST
EcalSelectiveReadoutValidation: Invalid iE value: 101
EcalSelectiveReadoutValidation: Invalid iE value: 101
EcalSelectiveReadoutValidation: Invalid iE value: 101
EcalSelectiveReadoutValidation: Invalid iE value: 101
There was a problem hiding this comment.
std::cout is not allowed in code that is run in production.
There was a problem hiding this comment.
I've copied the std::cout code from a different location in the same file.
Point was to fix the error with least changes to the code and the code style.
I agree that we shouldn't use std::cout, but currently it's all over the code.
I will discuss it further and make a separate pull request(s) converting those.
There was a problem hiding this comment.
thanks. this one is has been merged anyway. :)
... in Validation/EcalDigis.
Ignoring such values is ways better than modifying unrelated
variables on the stack.
Discussion: https://hypernews.cern.ch/HyperNews/CMS/get/swDevelopment/3202.html