Skip to content

Commit b70e1d5

Browse files
firewavefirewave
authored
avoid some unchecked pointer dereferences (danmar#4811)
1 parent 2151244 commit b70e1d5

32 files changed

Lines changed: 292 additions & 290 deletions

‎cli/cmdlineparser.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ static bool addFilesToList(const std::string& fileList, std::vector<std::string>
8181
return true;
8282
}
8383

84-
static bool addIncludePathsToList(const std::string& fileList, std::list<std::string>* pathNames)
84+
static bool addIncludePathsToList(const std::string& fileList, std::list<std::string>& pathNames)
8585
{
8686
std::ifstream files(fileList);
8787
if (files) {
@@ -96,20 +96,20 @@ static bool addIncludePathsToList(const std::string& fileList, std::list<std::st
9696
if (!endsWith(pathName, '/'))
9797
pathName += '/';
9898

99-
pathNames->emplace_back(std::move(pathName));
99+
pathNames.emplace_back(std::move(pathName));
100100
}
101101
}
102102
return true;
103103
}
104104
return false;
105105
}
106106

107-
static bool addPathsToSet(const std::string& fileName, std::set<std::string>* set)
107+
static bool addPathsToSet(const std::string& fileName, std::set<std::string>& set)
108108
{
109109
std::list<std::string> templist;
110-
if (!addIncludePathsToList(fileName, &templist))
110+
if (!addIncludePathsToList(fileName, templist))
111111
return false;
112-
set->insert(templist.cbegin(), templist.cend());
112+
set.insert(templist.cbegin(), templist.cend());
113113
return true;
114114
}
115115

@@ -279,7 +279,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
279279
else if (std::strncmp(argv[i], "--config-excludes-file=", 23) == 0) {
280280
// open this file and read every input file (1 file name per line)
281281
const std::string cfgExcludesFile(23 + argv[i]);
282-
if (!addPathsToSet(cfgExcludesFile, &mSettings->configExcludePaths)) {
282+
if (!addPathsToSet(cfgExcludesFile, mSettings->configExcludePaths)) {
283283
printError("unable to open config excludes file at '" + cfgExcludesFile + "'");
284284
return false;
285285
}
@@ -468,7 +468,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
468468
else if (std::strncmp(argv[i], "--includes-file=", 16) == 0) {
469469
// open this file and read every input file (1 file name per line)
470470
const std::string includesFile(16 + argv[i]);
471-
if (!addIncludePathsToList(includesFile, &mSettings->includePaths)) {
471+
if (!addIncludePathsToList(includesFile, mSettings->includePaths)) {
472472
printError("unable to open includes file at '" + includesFile + "'");
473473
return false;
474474
}

‎cli/cppcheckexecutor.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -568,9 +568,9 @@ bool CppCheckExecutor::tryLoadLibrary(Library& destination, const std::string& b
568568
*/
569569
// cppcheck-suppress passedByValue - used as callback so we need to preserve the signature
570570
// NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature
571-
bool CppCheckExecutor::executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string *output_)
571+
bool CppCheckExecutor::executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_)
572572
{
573-
output_->clear();
573+
output_.clear();
574574

575575
std::string joinedArgs;
576576
for (const std::string &arg : args) {
@@ -596,7 +596,7 @@ bool CppCheckExecutor::executeCommand(std::string exe, std::vector<std::string>
596596
return false;
597597
char buffer[1024];
598598
while (fgets(buffer, sizeof(buffer), pipe.get()) != nullptr)
599-
*output_ += buffer;
599+
output_ += buffer;
600600
return true;
601601
}
602602

‎cli/cppcheckexecutor.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ class CppCheckExecutor : public ErrorLogger {
114114
/**
115115
* Execute a shell command and read the output from it. Returns true if command terminated successfully.
116116
*/
117-
static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string *output_);
117+
static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_);
118118

119119
static bool reportSuppressions(const Settings &settings, bool unusedFunctionCheckEnabled, const std::map<std::string, std::size_t> &files, ErrorLogger& errorLogger);
120120

‎gui/checkthread.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@
4545
#include <QSettings>
4646

4747
// NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature
48-
static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string *output)
48+
static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output)
4949
{
50-
output->clear();
50+
output.clear();
5151

5252
QStringList args2;
5353
for (const std::string &arg: args)
@@ -60,9 +60,9 @@ static bool executeCommand(std::string exe, std::vector<std::string> args, std::
6060
if (redirect == "2>&1") {
6161
QString s1 = process.readAllStandardOutput();
6262
QString s2 = process.readAllStandardError();
63-
*output = (s1 + "\n" + s2).toStdString();
63+
output = (s1 + "\n" + s2).toStdString();
6464
} else
65-
*output = process.readAllStandardOutput().toStdString();
65+
output = process.readAllStandardOutput().toStdString();
6666

6767
if (redirect.compare(0,3,"2> ") == 0) {
6868
std::ofstream fout(redirect.substr(3));

‎lib/analyzerinfo.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void AnalyzerInformation::close()
7272
}
7373
}
7474

75-
static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash, std::list<ErrorMessage> *errors)
75+
static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash, std::list<ErrorMessage> &errors)
7676
{
7777
tinyxml2::XMLDocument doc;
7878
const tinyxml2::XMLError error = doc.LoadFile(analyzerInfoFile.c_str());
@@ -89,7 +89,7 @@ static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash,
8989

9090
for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
9191
if (std::strcmp(e->Name(), "error") == 0)
92-
errors->emplace_back(e);
92+
errors.emplace_back(e);
9393
}
9494

9595
return true;
@@ -127,7 +127,7 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir
127127
return Path::join(buildDir, filename) + ".analyzerinfo";
128128
}
129129

130-
bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t hash, std::list<ErrorMessage> *errors)
130+
bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t hash, std::list<ErrorMessage> &errors)
131131
{
132132
if (buildDir.empty() || sourcefile.empty())
133133
return true;

‎lib/analyzerinfo.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class CPPCHECKLIB AnalyzerInformation {
5555

5656
/** Close current TU.analyzerinfo file */
5757
void close();
58-
bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t hash, std::list<ErrorMessage> *errors);
58+
bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t hash, std::list<ErrorMessage> &errors);
5959
void reportErr(const ErrorMessage &msg);
6060
void setFileInfo(const std::string &check, const std::string &fileInfo);
6161
static std::string getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg);

‎lib/astutils.cpp‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static int getArgumentPos(const Token* ftok, const Token* tokToFind){
109109
}
110110

111111
template<class T, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
112-
static void astFlattenRecursive(T* tok, std::vector<T*>* result, const char* op, nonneg int depth = 0)
112+
static void astFlattenRecursive(T* tok, std::vector<T*>& result, const char* op, nonneg int depth = 0)
113113
{
114114
++depth;
115115
if (!tok || depth >= 100)
@@ -118,21 +118,21 @@ static void astFlattenRecursive(T* tok, std::vector<T*>* result, const char* op,
118118
astFlattenRecursive(tok->astOperand1(), result, op, depth);
119119
astFlattenRecursive(tok->astOperand2(), result, op, depth);
120120
} else {
121-
result->push_back(tok);
121+
result.push_back(tok);
122122
}
123123
}
124124

125125
std::vector<const Token*> astFlatten(const Token* tok, const char* op)
126126
{
127127
std::vector<const Token*> result;
128-
astFlattenRecursive(tok, &result, op);
128+
astFlattenRecursive(tok, result, op);
129129
return result;
130130
}
131131

132132
std::vector<Token*> astFlatten(Token* tok, const char* op)
133133
{
134134
std::vector<Token*> result;
135-
astFlattenRecursive(tok, &result, op);
135+
astFlattenRecursive(tok, result, op);
136136
return result;
137137
}
138138

@@ -865,12 +865,12 @@ const Token *findNextTokenFromBreak(const Token *breakToken)
865865
}
866866

867867
bool extractForLoopValues(const Token *forToken,
868-
nonneg int * const varid,
869-
bool * const knownInitValue,
870-
MathLib::bigint * const initValue,
871-
bool * const partialCond,
872-
MathLib::bigint * const stepValue,
873-
MathLib::bigint * const lastValue)
868+
nonneg int &varid,
869+
bool &knownInitValue,
870+
MathLib::bigint &initValue,
871+
bool &partialCond,
872+
MathLib::bigint &stepValue,
873+
MathLib::bigint &lastValue)
874874
{
875875
if (!Token::simpleMatch(forToken, "for (") || !Token::simpleMatch(forToken->next()->astOperand2(), ";"))
876876
return false;
@@ -880,28 +880,28 @@ bool extractForLoopValues(const Token *forToken,
880880
if (!initExpr || !initExpr->isBinaryOp() || initExpr->str() != "=" || !Token::Match(initExpr->astOperand1(), "%var%"))
881881
return false;
882882
std::vector<MathLib::bigint> minInitValue = getMinValue(ValueFlow::makeIntegralInferModel(), initExpr->astOperand2()->values());
883-
*varid = initExpr->astOperand1()->varId();
884-
*knownInitValue = initExpr->astOperand2()->hasKnownIntValue();
885-
*initValue = minInitValue.empty() ? 0 : minInitValue.front();
886-
*partialCond = Token::Match(condExpr, "%oror%|&&");
883+
varid = initExpr->astOperand1()->varId();
884+
knownInitValue = initExpr->astOperand2()->hasKnownIntValue();
885+
initValue = minInitValue.empty() ? 0 : minInitValue.front();
886+
partialCond = Token::Match(condExpr, "%oror%|&&");
887887
visitAstNodes(condExpr, [varid, &condExpr](const Token *tok) {
888888
if (Token::Match(tok, "%oror%|&&"))
889889
return ChildrenToVisit::op1_and_op2;
890-
if (Token::Match(tok, "<|<=") && tok->isBinaryOp() && tok->astOperand1()->varId() == *varid && tok->astOperand2()->hasKnownIntValue()) {
890+
if (Token::Match(tok, "<|<=") && tok->isBinaryOp() && tok->astOperand1()->varId() == varid && tok->astOperand2()->hasKnownIntValue()) {
891891
if (Token::Match(condExpr, "%oror%|&&") || tok->astOperand2()->getKnownIntValue() < condExpr->astOperand2()->getKnownIntValue())
892892
condExpr = tok;
893893
}
894894
return ChildrenToVisit::none;
895895
});
896-
if (!Token::Match(condExpr, "<|<=") || !condExpr->isBinaryOp() || condExpr->astOperand1()->varId() != *varid || !condExpr->astOperand2()->hasKnownIntValue())
896+
if (!Token::Match(condExpr, "<|<=") || !condExpr->isBinaryOp() || condExpr->astOperand1()->varId() != varid || !condExpr->astOperand2()->hasKnownIntValue())
897897
return false;
898-
if (!incExpr || !incExpr->isUnaryOp("++") || incExpr->astOperand1()->varId() != *varid)
898+
if (!incExpr || !incExpr->isUnaryOp("++") || incExpr->astOperand1()->varId() != varid)
899899
return false;
900-
*stepValue = 1;
900+
stepValue = 1;
901901
if (condExpr->str() == "<")
902-
*lastValue = condExpr->astOperand2()->getKnownIntValue() - 1;
902+
lastValue = condExpr->astOperand2()->getKnownIntValue() - 1;
903903
else
904-
*lastValue = condExpr->astOperand2()->getKnownIntValue();
904+
lastValue = condExpr->astOperand2()->getKnownIntValue();
905905
return true;
906906
}
907907

‎lib/astutils.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,12 @@ const Token *findNextTokenFromBreak(const Token *breakToken);
216216
* Extract for loop values: loopvar varid, init value, step value, last value (inclusive)
217217
*/
218218
bool extractForLoopValues(const Token *forToken,
219-
nonneg int * const varid,
220-
bool * const knownInitValue,
221-
long long * const initValue,
222-
bool * const partialCond,
223-
long long * const stepValue,
224-
long long * const lastValue);
219+
nonneg int &varid,
220+
bool &knownInitValue,
221+
long long &initValue,
222+
bool &partialCond,
223+
long long &stepValue,
224+
long long &lastValue);
225225

226226
bool precedes(const Token * tok1, const Token * tok2);
227227
bool succeeds(const Token* tok1, const Token* tok2);

‎lib/checkbufferoverrun.cpp‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -183,47 +183,46 @@ static int getMinFormatStringOutputLength(const std::vector<const Token*> &param
183183

184184
//---------------------------------------------------------------------------
185185

186-
static bool getDimensionsEtc(const Token * const arrayToken, const Settings *settings, std::vector<Dimension> * const dimensions, ErrorPath * const errorPath, bool * const mightBeLarger, MathLib::bigint* path)
186+
static bool getDimensionsEtc(const Token * const arrayToken, const Settings *settings, std::vector<Dimension> &dimensions, ErrorPath &errorPath, bool &mightBeLarger, MathLib::bigint &path)
187187
{
188188
const Token *array = arrayToken;
189189
while (Token::Match(array, ".|::"))
190190
array = array->astOperand2();
191191

192192
if (array->variable() && array->variable()->isArray() && !array->variable()->dimensions().empty()) {
193-
*dimensions = array->variable()->dimensions();
194-
if (dimensions->size() >= 1 && ((*dimensions)[0].num <= 1 || !(*dimensions)[0].tok)) {
193+
dimensions = array->variable()->dimensions();
194+
if (dimensions[0].num <= 1 || !dimensions[0].tok) {
195195
visitAstNodes(arrayToken,
196196
[&](const Token *child) {
197197
if (child->originalName() == "->") {
198-
*mightBeLarger = true;
198+
mightBeLarger = true;
199199
return ChildrenToVisit::none;
200200
}
201201
return ChildrenToVisit::op1_and_op2;
202202
});
203203
}
204-
} else if (const Token *stringLiteral = array->getValueTokenMinStrSize(settings, path)) {
204+
} else if (const Token *stringLiteral = array->getValueTokenMinStrSize(settings, &path)) {
205205
Dimension dim;
206206
dim.tok = nullptr;
207207
dim.num = Token::getStrArraySize(stringLiteral);
208208
dim.known = array->hasKnownValue();
209-
dimensions->emplace_back(dim);
209+
dimensions.emplace_back(dim);
210210
} else if (array->valueType() && array->valueType()->pointer >= 1 && (array->valueType()->isIntegral() || array->valueType()->isFloat())) {
211211
const ValueFlow::Value *value = getBufferSizeValue(array);
212212
if (!value)
213213
return false;
214-
if (path)
215-
*path = value->path;
216-
*errorPath = value->errorPath;
214+
path = value->path;
215+
errorPath = value->errorPath;
217216
Dimension dim;
218217
dim.known = value->isKnown();
219218
dim.tok = nullptr;
220219
const int typeSize = array->valueType()->typeSize(*settings, array->valueType()->pointer > 1);
221220
if (typeSize == 0)
222221
return false;
223222
dim.num = value->intvalue / typeSize;
224-
dimensions->emplace_back(dim);
223+
dimensions.emplace_back(dim);
225224
}
226-
return !dimensions->empty();
225+
return !dimensions.empty();
227226
}
228227

229228
static ValueFlow::Value makeSizeValue(MathLib::bigint size, MathLib::bigint path)
@@ -314,7 +313,7 @@ void CheckBufferOverrun::arrayIndex()
314313
ErrorPath errorPath;
315314
bool mightBeLarger = false;
316315
MathLib::bigint path = 0;
317-
if (!getDimensionsEtc(tok->astOperand1(), mSettings, &dimensions, &errorPath, &mightBeLarger, &path))
316+
if (!getDimensionsEtc(tok->astOperand1(), mSettings, dimensions, errorPath, mightBeLarger, path))
318317
continue;
319318

320319
const Variable* const var = array->variable();
@@ -486,7 +485,7 @@ void CheckBufferOverrun::pointerArithmetic()
486485
ErrorPath errorPath;
487486
bool mightBeLarger = false;
488487
MathLib::bigint path = 0;
489-
if (!getDimensionsEtc(arrayToken, mSettings, &dimensions, &errorPath, &mightBeLarger, &path))
488+
if (!getDimensionsEtc(arrayToken, mSettings, dimensions, errorPath, mightBeLarger, path))
490489
continue;
491490

492491
if (tok->str() == "+") {
@@ -881,6 +880,8 @@ std::string CheckBufferOverrun::MyFileInfo::toString() const
881880

882881
bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *offset, int type)
883882
{
883+
if (!offset)
884+
return false;
884885
const CheckBufferOverrun *c = dynamic_cast<const CheckBufferOverrun *>(check);
885886
if (!c)
886887
return false;
@@ -897,8 +898,6 @@ bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Check *check, const Token
897898
return false;
898899
if (!indexTok->hasKnownIntValue())
899900
return false;
900-
if (!offset)
901-
return false;
902901
*offset = indexTok->getKnownIntValue() * argtok->valueType()->typeSize(*c->mSettings);
903902
return true;
904903
}

0 commit comments

Comments
 (0)