Fixed static analyzer warnings #11097
Conversation
|
A new Pull Request was created by @civanch (Vladimir Ivantchenko) for CMSSW_7_6_X. Fixed static analyzer warnings It involves the following packages: SimCalorimetry/EcalSelectiveReadoutAlgos @cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Ok I have restarted the tests |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
cmsbuild
added
simulation-approved
fully-signed
and removed
pending-signatures
simulation-pending
labels
| @@ -222,7 +207,7 @@ double SiG4UniversalFluctuation::SampleFluctuations(const double momentum, | |||
| // | |||
| if (suma > sumalim) | |||
| { | |||
| p1 = 0., p2 = 0 ; | |||
| double p1, p2; | |||
There was a problem hiding this comment.
@civanch - you need the initialization here, no?
There was a problem hiding this comment.
Hi David, no, because both p1 and p2 are defined below in any case. The static analyser complains to this line.
There was a problem hiding this comment.
I see if (something) p1=blah then if (something2) p2=blah, then bah=p1_a+p2_b. So you definitely need to initialize p1 and p2.
There was a problem hiding this comment.
HI David, the full block:
double p1, p2;
if((a1+a2) > 0.)
{
// excitation type 1
if (a1>alim) {
siga=sqrt(a1) ;
p1 = max(0., CLHEP::RandGaussQ::shoot(engine, a1, siga) + 0.5);
} else {
p1 = double(CLHEP::RandPoissonQ::shoot(engine,a1));
}
// excitation type 2
if (a2>alim) {
siga=sqrt(a2) ;
p2 = max(0., CLHEP::RandGaussQ::shoot(engine, a2, siga) + 0.5);
} else {
p2 = double(CLHEP::RandPoissonQ::shoot(engine,a2));
}
loss = p1*e1Fluct+p2*e2Fluct;
// smearing to avoid unphysical peaks
if (p2 > 0.)
loss += (1.-2.*CLHEP::RandFlat::shoot(engine))*e2Fluct;
else if (loss>0.)
loss += (1.-2.*CLHEP::RandFlat::shoot(engine))*e1Fluct;
if (loss < 0.) loss = 0.0;
}
so, for both p1 and p2 we have if()p1=value1; else p1=value2, the same for p2 - both are defined in any case.
However, what I see now: the line 'double p1, p2;' may be moved down inside the block. Minor thing, should I do this?
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs or unless it breaks tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
please test |
|
The tests are being triggered in jenkins. |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs or unless it breaks tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
ah -whoops - thanks @civanch |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
+1 |
Fixed static analyzer warnings
The most of fixes are trivial and fixes are obvious. Few places may provide crash. For example, RunManagerMTWorker in abnormal case may crash at exit, which will shadow real problem.