From 3a6c8e1acd71eb4e14e76b2878558c19aa010a55 Mon Sep 17 00:00:00 2001
From: Nick Draper <nick.draper@stfc.ac.uk>
Date: Tue, 13 Oct 2015 11:13:43 +0100
Subject: [PATCH] Fix various Coverity control flow issues
re #13938
---
Framework/API/src/ExperimentInfo.cpp | 5 +-
Framework/API/src/WorkspaceOpOverloads.cpp | 51 ++++++++-----------
Framework/Crystal/src/LoadIsawSpectrum.cpp | 48 +++++++----------
Framework/Crystal/src/SaveHKL.cpp | 44 ++++++----------
.../src/Algorithms/FitPowderDiffPeaks.cpp | 3 +-
.../CurveFitting/src/Algorithms/LeBailFit.cpp | 2 +-
.../src/Algorithms/SplineBackground.cpp | 3 +-
.../DataHandling/src/GroupDetectors2.cpp | 4 +-
Framework/DataObjects/src/EventList.cpp | 4 +-
.../InstrumentWidget/UnwrappedSurface.cpp | 4 +-
.../src/Indirect/ApplyPaalmanPings.cpp | 1 +
.../src/Tomography/TomographyIfaceModel.cpp | 1 -
12 files changed, 65 insertions(+), 105 deletions(-)
diff --git a/Framework/API/src/ExperimentInfo.cpp b/Framework/API/src/ExperimentInfo.cpp
index 26a61864f6b..7f5f9dc9597 100644
--- a/Framework/API/src/ExperimentInfo.cpp
+++ b/Framework/API/src/ExperimentInfo.cpp
@@ -1149,7 +1149,7 @@ void ExperimentInfo::readParameterMap(const std::string ¶meterStr) {
// if( comp_name == prev_name ) continue; this blocks reading in different
// parameters of the same component. RNT
// prev_name = comp_name;
- const Geometry::IComponent *comp = 0;
+ const Geometry::IComponent *comp = nullptr;
if (comp_name.find("detID:") != std::string::npos) {
int detID = atoi(comp_name.substr(6).c_str());
comp = instr->getDetector(detID).get();
@@ -1164,8 +1164,7 @@ void ExperimentInfo::readParameterMap(const std::string ¶meterStr) {
continue;
}
}
- if (!comp)
- continue;
+
// create parameter's value as a sum of all tokens with index 3 or larger
// this allow a parameter's value to contain ";"
std::string paramValue = tokens[3];
diff --git a/Framework/API/src/WorkspaceOpOverloads.cpp b/Framework/API/src/WorkspaceOpOverloads.cpp
index cfd3595724a..b333686ed2b 100644
--- a/Framework/API/src/WorkspaceOpOverloads.cpp
+++ b/Framework/API/src/WorkspaceOpOverloads.cpp
@@ -5,11 +5,11 @@
#include "MantidAPI/Algorithm.h"
#include "MantidAPI/AlgorithmManager.h"
#include "MantidKernel/Property.h"
-#include "MantidKernel/Exception.h"
+//#include "MantidKernel/Exception.h"
#include "MantidAPI/MatrixWorkspace.h"
#include "MantidAPI/IWorkspaceProperty.h"
#include "MantidAPI/WorkspaceFactory.h"
-#include "MantidAPI/SpectraAxis.h"
+//#include "MantidAPI/SpectraAxis.h"
#include "MantidAPI/IMDWorkspace.h"
#include "MantidAPI/IMDHistoWorkspace.h"
#include "MantidAPI/WorkspaceGroup_fwd.h"
@@ -67,30 +67,21 @@ ResultType executeBinaryOperation(const std::string &algorithmName,
alg->execute();
- if (alg->isExecuted()) {
- // Get the output workspace property
- if (child) {
- return alg->getProperty("OutputWorkspace");
- } else {
- API::Workspace_sptr result =
- API::AnalysisDataService::Instance().retrieve(
- alg->getPropertyValue("OutputWorkspace"));
- return boost::dynamic_pointer_cast<typename ResultType::element_type>(
- result);
- }
- } else {
+ if (!alg->isExecuted()) {
std::string message = "Error while executing operation: " + algorithmName;
throw std::runtime_error(message);
}
- throw Kernel::Exception::NotFoundError(
- "Required output workspace property not found on Child Algorithm",
- "OutputWorkspace");
-
- // Horendous code inclusion to satisfy compilers that all code paths return a
- // value
- // in reality the above code should either throw or return successfully.
- return ResultType();
+ // Get the output workspace property
+ if (child) {
+ return alg->getProperty("OutputWorkspace");
+ } else {
+ API::Workspace_sptr result =
+ API::AnalysisDataService::Instance().retrieve(
+ alg->getPropertyValue("OutputWorkspace"));
+ return boost::dynamic_pointer_cast<typename ResultType::element_type>(
+ result);
+ }
}
template DLLExport MatrixWorkspace_sptr
@@ -169,14 +160,12 @@ bool equals(const MatrixWorkspace_sptr lhs, const MatrixWorkspace_sptr rhs,
// Rest: use default
alg->execute();
- if (alg->isExecuted()) {
- return (alg->getPropertyValue("Result") == "Success!");
- } else {
+ if (!alg->isExecuted()) {
std::string message =
"Error while executing operation: CheckWorkspacesMatch";
throw std::runtime_error(message);
}
- return false;
+ return (alg->getPropertyValue("Result") == "Success!");
}
/** Creates a temporary single value workspace the error is set to zero
@@ -512,17 +501,17 @@ bool WorkspaceHelpers::matchingBins(const MatrixWorkspace_const_sptr ws1,
if (!step)
step = 1;
for (size_t i = step; i < numHist; i += step) {
- const double firstWS =
+ const double firstWSLoop =
std::accumulate(ws1->readX(i).begin(), ws1->readX(i).end(), 0.);
- const double secondWS =
+ const double secondWSLoop =
std::accumulate(ws2->readX(i).begin(), ws2->readX(i).end(), 0.);
- if (std::abs(firstWS) < 1.0E-7 && std::abs(secondWS) < 1.0E-7) {
+ if (std::abs(firstWSLoop) < 1.0E-7 && std::abs(secondWSLoop) < 1.0E-7) {
for (size_t j = 0; j < ws1->readX(i).size(); j++) {
if (std::abs(ws1->readX(i)[j] - ws2->readX(i)[j]) > 1.0E-7)
return false;
}
- } else if (std::abs(firstWS - secondWS) /
- std::max<double>(std::abs(firstWS), std::abs(secondWS)) >
+ } else if (std::abs(firstWSLoop - secondWSLoop) /
+ std::max<double>(std::abs(firstWSLoop), std::abs(secondWSLoop)) >
1.0E-7)
return false;
}
diff --git a/Framework/Crystal/src/LoadIsawSpectrum.cpp b/Framework/Crystal/src/LoadIsawSpectrum.cpp
index 1020210a59d..20e26e01cda 100644
--- a/Framework/Crystal/src/LoadIsawSpectrum.cpp
+++ b/Framework/Crystal/src/LoadIsawSpectrum.cpp
@@ -69,38 +69,24 @@ void LoadIsawSpectrum::exec() {
std::vector<std::vector<double>> spectra;
std::vector<std::vector<double>> time;
int iSpec = 0;
- if (iSpec == 1) {
- while (!infile.eof()) // To get you all the lines.
- {
- // Set up sizes. (HEIGHT x WIDTH)
- spectra.resize(a + 1);
- getline(infile, STRING); // Saves the line in STRING.
- infile >> spec[0] >> spec[1] >> spec[2] >> spec[3] >> spec[4] >>
- spec[5] >> spec[6] >> spec[7] >> spec[8] >> spec[9] >> spec[10];
- for (int i = 0; i < 11; i++)
- spectra[a].push_back(spec[i]);
- a++;
- }
- } else {
- for (int wi = 0; wi < 8; wi++)
- getline(infile, STRING); // Saves the line in STRING.
- while (!infile.eof()) // To get you all the lines.
- {
- time.resize(a + 1);
- spectra.resize(a + 1);
- getline(infile, STRING); // Saves the line in STRING.
- if (infile.eof())
- break;
- std::stringstream ss(STRING);
- if (STRING.find("Bank") == std::string::npos) {
- double time0, spectra0;
- ss >> time0 >> spectra0;
- time[a].push_back(time0);
- spectra[a].push_back(spectra0);
+ for (int wi = 0; wi < 8; wi++)
+ getline(infile, STRING); // Saves the line in STRING.
+ while (!infile.eof()) // To get you all the lines.
+ {
+ time.resize(a + 1);
+ spectra.resize(a + 1);
+ getline(infile, STRING); // Saves the line in STRING.
+ if (infile.eof())
+ break;
+ std::stringstream ss(STRING);
+ if (STRING.find("Bank") == std::string::npos) {
+ double time0, spectra0;
+ ss >> time0 >> spectra0;
+ time[a].push_back(time0);
+ spectra[a].push_back(spectra0);
- } else {
- a++;
- }
+ } else {
+ a++;
}
}
infile.close();
diff --git a/Framework/Crystal/src/SaveHKL.cpp b/Framework/Crystal/src/SaveHKL.cpp
index 531893ebf51..3129a148080 100644
--- a/Framework/Crystal/src/SaveHKL.cpp
+++ b/Framework/Crystal/src/SaveHKL.cpp
@@ -230,37 +230,23 @@ void SaveHKL::exec() {
infile.open(spectraFile.c_str());
if (infile.is_open()) {
size_t a = 0;
- if (iSpec == 1) {
- while (!infile.eof()) // To get you all the lines.
- {
- // Set up sizes. (HEIGHT x WIDTH)
- spectra.resize(a + 1);
- getline(infile, STRING); // Saves the line in STRING.
- infile >> spec[0] >> spec[1] >> spec[2] >> spec[3] >> spec[4] >>
- spec[5] >> spec[6] >> spec[7] >> spec[8] >> spec[9] >> spec[10];
- for (int i = 0; i < 11; i++)
- spectra[a].push_back(spec[i]);
+ for (int wi = 0; wi < 8; wi++)
+ getline(infile, STRING); // Saves the line in STRING.
+ while (!infile.eof()) // To get you all the lines.
+ {
+ time.resize(a + 1);
+ spectra.resize(a + 1);
+ getline(infile, STRING); // Saves the line in STRING.
+ std::stringstream ss(STRING);
+ if (STRING.find("Bank") == std::string::npos) {
+ double time0, spectra0;
+ ss >> time0 >> spectra0;
+ time[a].push_back(time0);
+ spectra[a].push_back(spectra0);
+
+ } else {
a++;
}
- } else {
- for (int wi = 0; wi < 8; wi++)
- getline(infile, STRING); // Saves the line in STRING.
- while (!infile.eof()) // To get you all the lines.
- {
- time.resize(a + 1);
- spectra.resize(a + 1);
- getline(infile, STRING); // Saves the line in STRING.
- std::stringstream ss(STRING);
- if (STRING.find("Bank") == std::string::npos) {
- double time0, spectra0;
- ss >> time0 >> spectra0;
- time[a].push_back(time0);
- spectra[a].push_back(spectra0);
-
- } else {
- a++;
- }
- }
}
infile.close();
}
diff --git a/Framework/CurveFitting/src/Algorithms/FitPowderDiffPeaks.cpp b/Framework/CurveFitting/src/Algorithms/FitPowderDiffPeaks.cpp
index b69b92fdba2..87801460543 100644
--- a/Framework/CurveFitting/src/Algorithms/FitPowderDiffPeaks.cpp
+++ b/Framework/CurveFitting/src/Algorithms/FitPowderDiffPeaks.cpp
@@ -2021,7 +2021,6 @@ bool FitPowderDiffPeaks::doFitMultiplePeaks(
// 1. Fit peaks intensities first
const size_t numpeaks = peakfuncs.size();
map<string, double> peaksfuncparams;
- bool evergood = true;
// a) Set up fit/fix
vector<string> peakparnames = peakfuncs[0]->getParameterNames();
@@ -2045,7 +2044,7 @@ bool FitPowderDiffPeaks::doFitMultiplePeaks(
double chi2;
bool fitgood = doFitNPeaksSimple(dataws, wsindex, peaksfunc, peakfuncs,
"Levenberg-MarquardtMD", 1000, chi2);
- evergood = evergood || fitgood;
+ bool evergood = fitgood;
// c) Process result
if (!fitgood) {
diff --git a/Framework/CurveFitting/src/Algorithms/LeBailFit.cpp b/Framework/CurveFitting/src/Algorithms/LeBailFit.cpp
index 60e3d731b21..2977bb77d42 100644
--- a/Framework/CurveFitting/src/Algorithms/LeBailFit.cpp
+++ b/Framework/CurveFitting/src/Algorithms/LeBailFit.cpp
@@ -341,7 +341,7 @@ void LeBailFit::exec() {
case FIT:
// LeBail Fit
g_log.notice() << "Function: Do LeBail Fit ==> Monte Carlo.\n";
-
+ //fall through
case MONTECARLO:
// Monte carlo Le Bail refinement
g_log.notice("Function: Do LeBail Fit By Monte Carlo Random Walk.");
diff --git a/Framework/CurveFitting/src/Algorithms/SplineBackground.cpp b/Framework/CurveFitting/src/Algorithms/SplineBackground.cpp
index 93b2e86016a..a19fa510610 100644
--- a/Framework/CurveFitting/src/Algorithms/SplineBackground.cpp
+++ b/Framework/CurveFitting/src/Algorithms/SplineBackground.cpp
@@ -66,8 +66,7 @@ void SplineBackground::exec() {
bool isMasked = inWS->hasMaskedBins(spec);
std::vector<int> masked(Y.size());
if (isMasked) {
- for (API::MatrixWorkspace::MaskList::const_iterator it =
- inWS->maskedBins(spec).begin();
+ for (auto it = inWS->maskedBins(spec).begin();
it != inWS->maskedBins(spec).end(); ++it)
masked[it->first] = 1;
n -= static_cast<int>(inWS->maskedBins(spec).size());
diff --git a/Framework/DataHandling/src/GroupDetectors2.cpp b/Framework/DataHandling/src/GroupDetectors2.cpp
index 060f54bee77..4bfba12fc4a 100644
--- a/Framework/DataHandling/src/GroupDetectors2.cpp
+++ b/Framework/DataHandling/src/GroupDetectors2.cpp
@@ -353,7 +353,7 @@ void GroupDetectors2::getGroups(API::MatrixWorkspace_const_sptr workspace,
}
// check we don't have an index that is too high for the workspace
size_t maxIn = static_cast<size_t>(workspace->getNumberHistograms() - 1);
- std::vector<size_t>::const_iterator it = m_GroupSpecInds[0].begin();
+ auto it = m_GroupSpecInds[0].begin();
for (; it != m_GroupSpecInds[0].end(); ++it) {
if (*it > maxIn) {
g_log.error() << "Spectra index " << *it
@@ -375,7 +375,7 @@ void GroupDetectors2::getGroups(API::MatrixWorkspace_const_sptr workspace,
// up date unUsedSpec, this is used to find duplicates and when the user has
// set KeepUngroupedSpectra
- std::vector<size_t>::const_iterator index = m_GroupSpecInds[0].begin();
+ auto index = m_GroupSpecInds[0].begin();
for (; index != m_GroupSpecInds[0].end();
++index) { // the vector<int> m_GroupSpecInds[0] must not index contain
// numbers that don't exist in the workspaace
diff --git a/Framework/DataObjects/src/EventList.cpp b/Framework/DataObjects/src/EventList.cpp
index efb6e74c713..0afb9419bd2 100644
--- a/Framework/DataObjects/src/EventList.cpp
+++ b/Framework/DataObjects/src/EventList.cpp
@@ -528,7 +528,7 @@ EventList &EventList::operator-=(const EventList &more_events) {
this->clearData();
return *this;
}
-
+
// We'll let the -= operator for the given vector of event lists handle it
switch (this->getEventType()) {
case TOF:
@@ -548,6 +548,7 @@ EventList &EventList::operator-=(const EventList &more_events) {
minusHelper(this->weightedEvents, more_events.weightedEventsNoTime);
break;
}
+ break;
case WEIGHTED_NOTIME:
switch (more_events.getEventType()) {
@@ -561,6 +562,7 @@ EventList &EventList::operator-=(const EventList &more_events) {
minusHelper(this->weightedEventsNoTime, more_events.weightedEventsNoTime);
break;
}
+ break;
}
// No guaranteed order
diff --git a/MantidPlot/src/Mantid/InstrumentWidget/UnwrappedSurface.cpp b/MantidPlot/src/Mantid/InstrumentWidget/UnwrappedSurface.cpp
index d83662b41b3..77766f8eff2 100644
--- a/MantidPlot/src/Mantid/InstrumentWidget/UnwrappedSurface.cpp
+++ b/MantidPlot/src/Mantid/InstrumentWidget/UnwrappedSurface.cpp
@@ -581,8 +581,8 @@ void UnwrappedSurface::drawSimpleToImage(QImage* image,bool picking)const
if ( iw < 4 ) iw = 4;
if ( ih < 4 ) ih = 4;
- double w = (iw == 0)? dw : udet.width/2;
- double h = (ih == 0)? dh : udet.height/2;
+ double w = udet.width/2;
+ double h = udet.height/2;
if (!(m_viewRect.contains(udet.u-w, udet.v-h) || m_viewRect.contains(udet.u+w, udet.v+h))) continue;
diff --git a/MantidQt/CustomInterfaces/src/Indirect/ApplyPaalmanPings.cpp b/MantidQt/CustomInterfaces/src/Indirect/ApplyPaalmanPings.cpp
index 42270faab77..737204a16b7 100644
--- a/MantidQt/CustomInterfaces/src/Indirect/ApplyPaalmanPings.cpp
+++ b/MantidQt/CustomInterfaces/src/Indirect/ApplyPaalmanPings.cpp
@@ -145,6 +145,7 @@ void ApplyPaalmanPings::run() {
switch (result) {
case QMessageBox::YesToAll:
interpolateAll = true;
+ //fall through
case QMessageBox::Yes:
addInterpolationStep(factorWs, absCorProps["SampleWorkspace"]);
break;
diff --git a/MantidQt/CustomInterfaces/src/Tomography/TomographyIfaceModel.cpp b/MantidQt/CustomInterfaces/src/Tomography/TomographyIfaceModel.cpp
index 15c9f4550e6..5aaaaccde66 100644
--- a/MantidQt/CustomInterfaces/src/Tomography/TomographyIfaceModel.cpp
+++ b/MantidQt/CustomInterfaces/src/Tomography/TomographyIfaceModel.cpp
@@ -582,7 +582,6 @@ TomographyIfaceModel::loadFITSImage(const std::string &path) {
"Failed to load image. Could not load this file as a "
"FITS image: " +
std::string(e.what()));
- return WorkspaceGroup_sptr();
}
if (!alg->isExecuted()) {
throw std::runtime_error(
--
GitLab