Fixed Logic Errors in CalibrationXML.cc, SiStripElectronAlgo.cc & InOutConversionSeedFinder.cc #15575
Conversation
…bout a null C++ object pointer being called in line 96 and line 133
… object pointer was being called
…l C++ object pointer was being called
|
A new Pull Request was created by @uhussain (Usama Hussain) for CMSSW_8_1_X. It involves the following packages: RecoBTag/XMLCalibration @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are list here #13028 |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Hi, most of these asserts or logic statement modifications should be changed to exceptions. Generally they are in functions where the result of a null pointer at that given point is a major code error in packages upstream of the client code. |
| @@ -1149,7 +1149,7 @@ bool SiStripElectronAlgo::projectPhiBand(float chargeHypothesis, const reco::Sup | |||
| double reducedChi2 = (totalNumberOfHits > 2 ? chi2 / (totalNumberOfHits - 2) : 1e10); | |||
|
|
|||
| // Select this candidate if it passes minHits_ and maxReducedChi2_ cuts | |||
| if (totalNumberOfHits >= minHits_ && reducedChi2 <= maxReducedChi2_) { | |||
| if (innerhit != nullptr && totalNumberOfHits >= minHits_ && reducedChi2 <= maxReducedChi2_) { | |||
There was a problem hiding this comment.
@lgray: Do we really want an exception here? I wouldn't want to stop all processing just because some inner hit is null, unless that is absolutely necessary.
|
I think |
| @@ -205,6 +205,7 @@ void InOutConversionSeedFinder::fillClusterSeeds() const { | |||
| } | |||
|
|
|||
| //PropagatorWithMaterial reversePropagator(oppositeToMomentum, 0.000511, &(*theMF_) ); | |||
| assert(myPointer); | |||
There was a problem hiding this comment.
This assert could replaced with an exception like:
throw cms::Exception("Null measurement") << "Missing track trajectory measurement";
There was a problem hiding this comment.
We decided that, since this condition is so unlikely, a simple assert is good enough.
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar |
cmsbuild
added
reconstruction-approved
fully-signed
and removed
pending-signatures
reconstruction-pending
labels
|
+1 |
No description provided.