Skip to content
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.

Sign up for GitHub

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

[DQM] [Clang]Cleanup clang-analyzer warnings #46227

Merged
merged 1 commit into from
Oct 7, 2024
Merged

[DQM] [Clang]Cleanup clang-analyzer warnings #46227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Validation/CaloTowers/src/CaloTowersClient.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,13 @@ int CaloTowersClient::CaloTowersEndjob(const std::vector<MonitorElement*>& hcalM
if (useAllHistos != 0 && useAllHistos != 3)
return 0;

assert(mapEnergy_N);
double nevent = mapEnergy_N->getEntries();
if (verbose_)
std::cout << "nevent : " << nevent << std::endl;

// mean number of towers per ieta
assert(Ntowers_vs_ieta);
int nx = Ntowers_vs_ieta->getNbinsX();
float cont;
float econt;
Expand Down Expand Up @@ -139,6 +141,7 @@ int CaloTowersClient::CaloTowersEndjob(const std::vector<MonitorElement*>& hcalM
}

// Occupancy (needed for occupancy vs ieta)
assert(occupancy_map);
cnorm = occupancy_map->getBinContent(i, j) / fev;
enorm = occupancy_map->getBinError(i, j) / fev;
if (cnorm > 1.e-30)
Expand All @@ -163,6 +166,7 @@ int CaloTowersClient::CaloTowersEndjob(const std::vector<MonitorElement*>& hcalM

cnorm = sumphi / phi_factor;
enorm = sqrt(sumphie) / phi_factor;
assert(occupancy_vs_ieta);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cms-sw/dqm-l2 , may be there is better way to fix this code . All these pointers are not set in https://github.com/cms-sw/cmssw/blob/master/Validation/CaloTowers/src/CaloTowersClient.cc#L63-L87 block. So clang-analyzer warns that these might be null. For now I have added assert to instruct clang static analyzer that we fail if these are null

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your proposed solution for the null pointer error handling is fine.

occupancy_vs_ieta->setBinContent(i, cnorm);
occupancy_vs_ieta->setBinError(i, enorm);

Expand Down
2 changes: 2 additions & 0 deletions Validation/DTRecHits/plugins/DTRecHitQuality.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ void DTRecHitQuality::compute(const DTGeometry &dtGeom,
}

// Fill
assert(hRes);
hRes->fill(simHitWireDist,
simHitTheta,
simHitFEDist,
Expand Down Expand Up @@ -685,6 +686,7 @@ void DTRecHitQuality::compute(const DTGeometry &dtGeom,
}
}
// Fill
assert(hEff);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, just to make sure these pointers are not null

hEff->fill(simHitWireDist, simHitGlobalPos.eta(), simHitGlobalPos.phi(), recHitReconstructed);
if (hEffTot != nullptr) {
hEffTot->fill(simHitWireDist, simHitGlobalPos.eta(), simHitGlobalPos.phi(), recHitReconstructed);
Expand Down
1 change: 1 addition & 0 deletions Validation/EcalClusters/src/EnergyScaleAnalyzer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ void EnergyScaleAnalyzer::analyze(const edm::Event &evt, const edm::EventSetup &
Labels l;
labelsForToken(hepMCLabel_, l);

[[clang::suppress]]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

genEvent is used later in the code. Clang static analyzer complains that if hepMC.isValid() is not valid then genEvent is not used. [[clang::suppress]] here instruct clang analyzer to ignore this warnings

const HepMC::GenEvent *genEvent = hepMC->GetEvent();
if (!(hepMC.isValid())) {
LogInfo("EnergyScaleAnalyzer") << "Could not get MC Product!";
Expand Down
7 changes: 3 additions & 4 deletions Validation/EcalDigis/plugins/EcalBarrelDigisValidation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ void EcalBarrelDigisValidation::analyze(Event const& e, EventSetup const& c) {

void EcalBarrelDigisValidation::checkCalibrations(edm::EventSetup const& eventSetup) {
// ADC -> GeV Scale
[[clang::suppress]]
const EcalADCToGeVConstant* agc = &eventSetup.getData(pAgc);

EcalMGPAGainRatio* defaultRatios = new EcalMGPAGainRatio();
Expand All @@ -268,8 +269,6 @@ void EcalBarrelDigisValidation::checkCalibrations(edm::EventSetup const& eventSe

delete defaultRatios;

const double barrelADCtoGeV_ = agc->getEBValue();
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << barrelADCtoGeV_;
const double endcapADCtoGeV_ = agc->getEEValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << endcapADCtoGeV_;
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << agc->getEBValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << agc->getEEValue();
}
7 changes: 3 additions & 4 deletions Validation/EcalDigis/plugins/EcalDigisValidation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ void EcalDigisValidation::analyze(edm::Event const& e, edm::EventSetup const& c)

void EcalDigisValidation::checkCalibrations(edm::EventSetup const& eventSetup) {
// ADC -> GeV Scale
[[clang::suppress]]
const EcalADCToGeVConstant* agc = &eventSetup.getData(pAgc);

EcalMGPAGainRatio* defaultRatios = new EcalMGPAGainRatio();
Expand All @@ -397,8 +398,6 @@ void EcalDigisValidation::checkCalibrations(edm::EventSetup const& eventSetup) {

delete defaultRatios;

const double barrelADCtoGeV_ = agc->getEBValue();
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << barrelADCtoGeV_;
const double endcapADCtoGeV_ = agc->getEEValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << endcapADCtoGeV_;
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << agc->getEBValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << agc->getEEValue();
}
7 changes: 3 additions & 4 deletions Validation/EcalDigis/plugins/EcalEndcapDigisValidation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ void EcalEndcapDigisValidation::analyze(Event const& e, EventSetup const& c) {

void EcalEndcapDigisValidation::checkCalibrations(edm::EventSetup const& eventSetup) {
// ADC -> GeV Scale
[[clang::suppress]]
const EcalADCToGeVConstant* agc = &eventSetup.getData(pAgc);

EcalMGPAGainRatio* defaultRatios = new EcalMGPAGainRatio();
Expand All @@ -277,8 +278,6 @@ void EcalEndcapDigisValidation::checkCalibrations(edm::EventSetup const& eventSe

delete defaultRatios;

const double barrelADCtoGeV_ = agc->getEBValue();
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << barrelADCtoGeV_;
const double endcapADCtoGeV_ = agc->getEEValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << endcapADCtoGeV_;
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << agc->getEBValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << agc->getEEValue();
}
7 changes: 3 additions & 4 deletions Validation/EcalDigis/plugins/EcalMixingModuleValidation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,7 @@ void EcalMixingModuleValidation::analyze(edm::Event const& e, edm::EventSetup co

void EcalMixingModuleValidation::checkCalibrations(edm::EventSetup const& eventSetup) {
// ADC -> GeV Scale
[[clang::suppress]]
const EcalADCToGeVConstant* agc = &eventSetup.getData(pAgc);

EcalMGPAGainRatio defaultRatios;
Expand All @@ -627,10 +628,8 @@ void EcalMixingModuleValidation::checkCalibrations(edm::EventSetup const& eventS
<< " g2 = " << gainConv_[2] << "\n"
<< " g3 = " << gainConv_[3];

const double barrelADCtoGeV_ = agc->getEBValue();
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << barrelADCtoGeV_;
const double endcapADCtoGeV_ = agc->getEEValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << endcapADCtoGeV_;
LogDebug("EcalDigi") << " Barrel GeV/ADC = " << agc->getEBValue();
LogDebug("EcalDigi") << " Endcap GeV/ADC = " << agc->getEEValue();

// ES condition objects
const ESGain* esgain = &eventSetup.getData(esgain_);
Expand Down
3 changes: 0 additions & 3 deletions Validation/EcalDigis/plugins/EcalSelectiveReadoutValidation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2183,14 +2183,11 @@ void EcalSelectiveReadoutValidation::configFirWeights(const vector<double>& weig
log << weightsForZsFIR[i] << "\t";
}

double s2 = 0.;
log << "\nActual FIR weights: ";
for (unsigned i = 0; i < firWeights_.size(); ++i) {
log << firWeights_[i] << "\t";
s2 += firWeights_[i] * firWeights_[i];
}

s2 = sqrt(s2);
log << "\nNormalized FIR weights after hw representation rounding: ";
for (unsigned i = 0; i < firWeights_.size(); ++i) {
log << firWeights_[i] / (double)(1 << 10) << "\t";
Expand Down
4 changes: 2 additions & 2 deletions Validation/GlobalDigis/src/GlobalDigisAnalyzer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ void GlobalDigisAnalyzer::fillECal(const edm::Event &iEvent, const edm::EventSet
bool validDigiES = true;
if (!EcalDigiES.isValid()) {
LogDebug(MsgLoggerCat) << "Unable to find EcalDigiES in event!";
validDigiES = false;
[[clang::suppress]] validDigiES = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang analyzer complains as we force set validDigiES = false; at few line later

}

// ONLY WHILE GEOMETRY IS REMOVED
Expand Down Expand Up @@ -1468,7 +1468,7 @@ void GlobalDigisAnalyzer::fillMuon(const edm::Event &iEvent, const edm::EventSet
bool validrpcdigi = true;
if (!rpcDigis.isValid()) {
LogDebug(MsgLoggerCat) << "Unable to find rpcDigis in event!";
validrpcdigi = false;
[[clang::suppress]] validrpcdigi = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang analyzer complains as we force set validrpcdigi = false; at few line later

}

// ONLY UNTIL PROBLEM WITH RPC DIGIS IS FIGURED OUT
Expand Down
1 change: 1 addition & 0 deletions Validation/HLTrigger/plugins/HLTGenValSource.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ void HLTGenValSource::dqmBeginRun(const edm::Run& iRun, const edm::EventSetup& i
infoString_ += "\"date & time\":\"" + timeString + "\",";

// CMSSW version
[[clang::suppress]]
std::string cmsswVersion = std::getenv("CMSSW_VERSION");
infoString_ += std::string("\"CMSSW release\":\"") + cmsswVersion + "\",";

Expand Down
1 change: 0 additions & 1 deletion Validation/MuonDTDigis/src/MuonDTDigis.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ void MuonDTDigis::analyze(const Event &event, const EventSetup &eventSetup) {
const DTDigiCollection::Range &range = (*detUnitIt).second;

num_digis_layer = 0;
cham_num = 0;
wire_touched = 0;

// Loop over the digis of this DetUnit
Expand Down
2 changes: 1 addition & 1 deletion Validation/RecoB/plugins/BDHadronTrackMonitoringAnalyzer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ void BDHadronTrackMonitoringAnalyzer::analyze(const edm::Event &iEvent, const ed
sqrt(momentum_tpr.perp2()) * momentum_tpr.z() /
sqrt(momentum_tpr.perp2());

TrkTruthnHitAll = 0;
[[clang::suppress]] TrkTruthnHitAll = 0;
TrkTruthnHitPixel = 0;
TrkTruthnHitStrip = 0;
if (clusterRange.first != clusterRange.second) {
Expand Down
6 changes: 3 additions & 3 deletions Validation/RecoParticleFlow/src/Comparator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,15 +429,15 @@ void Comparator::Draw(TH1 *h0, TH1 *h1, Mode mode) {
std::ostringstream oss3;
oss3 << h1_->GetEntries();
const std::string txt_entries = "Entries = " + oss3.str();
text = ptstats2->AddText(txt_entries.c_str());
ptstats2->AddText(txt_entries.c_str());
std::ostringstream oss;
oss << h1_->GetMean();
const std::string txt_mean = "Mean = " + oss.str();
text = ptstats2->AddText(txt_mean.c_str());
ptstats2->AddText(txt_mean.c_str());
std::ostringstream oss2;
oss2 << h1_->GetRMS();
const std::string txt_rms = "RMS = " + oss2.str();
text = ptstats2->AddText(txt_rms.c_str());
ptstats2->AddText(txt_rms.c_str());
ptstats2->SetOptStat(1111);
ptstats2->SetOptFit(0);
ptstats2->Draw();
Expand Down