Skip to content

Commit 639f156

Browse files
PKEuSPKEuS
committed
Message refactorization: checkbufferoverrun.cpp (2), checkclass.cpp, checkexceptionsafety.h
1 parent 42e6855 commit 639f156

7 files changed

Lines changed: 148 additions & 141 deletions

‎lib/checkbufferoverrun.cpp‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ void CheckBufferOverrun::outOfBoundsError(const Token *tok, const std::string &w
142142
void CheckBufferOverrun::pointerOutOfBoundsError(const Token *tok, const std::string &object)
143143
{
144144
reportError(tok, Severity::portability, "pointerOutOfBounds", "Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the " + object + ".\n"
145-
"Undefined behaviour: Using pointer arithmetic so that the result does not point into or just past the end of the " + object + ". Further information: https://www.securecoding.cert.org/confluence/display/seccode/ARR30-C.+Do+not+form+or+use+out+of+bounds+pointers+or+array+subscripts");
145+
"Undefined behaviour: The result of this pointer arithmetic does not point into or just one element past the end of the " + object + ". Further information: https://www.securecoding.cert.org/confluence/display/seccode/ARR30-C.+Do+not+form+or+use+out+of+bounds+pointers+or+array+subscripts");
146146
}
147147

148148
void CheckBufferOverrun::sizeArgumentAsCharError(const Token *tok)
@@ -156,22 +156,22 @@ void CheckBufferOverrun::sizeArgumentAsCharError(const Token *tok)
156156
void CheckBufferOverrun::terminateStrncpyError(const Token *tok, const std::string &varname)
157157
{
158158
reportError(tok, Severity::warning, "terminateStrncpy",
159-
"The buffer '" + varname + "' may not be zero-terminated after the call to strncpy().\n"
159+
"The buffer '" + varname + "' may not be null-terminated after the call to strncpy().\n"
160160
"If the source string's size fits or exceeds the given size, strncpy() does not add a "
161161
"zero at the end of the buffer. This causes bugs later in the code if the code "
162-
"assumes buffer is zero-terminated.");
162+
"assumes buffer is null-terminated.");
163163
}
164164

165165
void CheckBufferOverrun::cmdLineArgsError(const Token *tok)
166166
{
167-
reportError(tok, Severity::error, "insecureCmdLineArgs", "Buffer overrun possible for long cmd-line args.");
167+
reportError(tok, Severity::error, "insecureCmdLineArgs", "Buffer overrun possible for long command line arguments.");
168168
}
169169

170170
void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const std::string &varname, const std::string &function)
171171
{
172-
const std::string errmsg = "The buffer '" + varname + "' is not zero-terminated after the call to " + function + "().\n"
173-
"The buffer '" + varname + "' is not zero-terminated after the call to " + function + "(). "
174-
"This will cause bugs later in the code if the code assumes the buffer is zero-terminated.";
172+
const std::string errmsg = "The buffer '" + varname + "' is not null-terminated after the call to " + function + "().\n"
173+
"The buffer '" + varname + "' is not null-terminated after the call to " + function + "(). "
174+
"This will cause bugs later in the code if the code assumes the buffer is null-terminated.";
175175

176176
reportError(tok, Severity::warning, "bufferNotZeroTerminated", errmsg, true);
177177
}
@@ -2110,7 +2110,8 @@ void CheckBufferOverrun::arrayIndexThenCheckError(const Token *tok, const std::s
21102110
{
21112111
reportError(tok, Severity::style, "arrayIndexThenCheck",
21122112
"Array index '" + indexName + "' is used before limits check.\n"
2113-
"Defensive programming: The variable '" + indexName + "' is used as array index and then there is a check that it is within limits. This can "
2114-
"mean that the array might be accessed out of bounds. Reorder conditions such as '(a[i] && i < 10)' to '(i < 10 && a[i])'. That way the "
2115-
"array will not be accessed if the index is out of limits.");
2113+
"Defensive programming: The variable '" + indexName + "' is used as an array index before it "
2114+
"is check that is within limits. This can mean that the array might be accessed out of bounds. "
2115+
"Reorder conditions such as '(a[i] && i < 10)' to '(i < 10 && a[i])'. That way the array will "
2116+
"not be accessed if the index is out of limits.");
21162117
}

‎lib/checkclass.cpp‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -481,8 +481,9 @@ void CheckClass::noConstructorError(const Token *tok, const std::string &classna
481481
"The " + std::string(isStruct ? "struct" : "class") + " '" + classname +
482482
"' does not have a constructor.\n"
483483
"The " + std::string(isStruct ? "struct" : "class") + " '" + classname +
484-
" 'does not have a constructor but it has attributes. "
485-
"The attributes are not initialized which may cause bugs or undefined behavior.");
484+
"' does not have a constructor although it has private member variables. "
485+
"Member variables of builtin types are left uninitialized when the class is "
486+
"instanciated. That may cause bugs or undefined behavior.");
486487
}
487488

488489
void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname)
@@ -492,7 +493,7 @@ void CheckClass::uninitVarError(const Token *tok, const std::string &classname,
492493

493494
void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname)
494495
{
495-
reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator=" + "'");
496+
reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator='.");
496497
}
497498

498499
//---------------------------------------------------------------------------
@@ -657,7 +658,7 @@ void CheckClass::privateFunctions()
657658

658659
void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname)
659660
{
660-
reportError(tok, Severity::style, "unusedPrivateFunction", "Unused private function '" + classname + "::" + funcname + "'");
661+
reportError(tok, Severity::style, "unusedPrivateFunction", "Unused private function: '" + classname + "::" + funcname + "'");
661662
}
662663

663664
//---------------------------------------------------------------------------
@@ -749,7 +750,7 @@ void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Sco
749750

750751
void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type)
751752
{
752-
reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a " + classname);
753+
reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a " + classname + ".");
753754
}
754755

755756
//---------------------------------------------------------------------------
@@ -790,7 +791,7 @@ void CheckClass::operatorEq()
790791

791792
void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className)
792793
{
793-
reportError(tok, Severity::style, "operatorEq", "\'" + className + "::operator=' should return \'" + className + " &\'");
794+
reportError(tok, Severity::style, "operatorEq", "'" + className + "::operator=' should return '" + className + " &'.");
794795
}
795796

796797
//---------------------------------------------------------------------------
@@ -880,7 +881,7 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
880881

881882
void CheckClass::operatorEqRetRefThisError(const Token *tok)
882883
{
883-
reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to self");
884+
reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to 'this' instance.");
884885
}
885886

886887
//---------------------------------------------------------------------------
@@ -1003,7 +1004,10 @@ bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs)
10031004

10041005
void CheckClass::operatorEqToSelfError(const Token *tok)
10051006
{
1006-
reportError(tok, Severity::warning, "operatorEqToSelf", "'operator=' should check for assignment to self");
1007+
reportError(tok, Severity::warning, "operatorEqToSelf",
1008+
"'operator=' should check for assignment to self to avoid problems with dynamic memory.\n"
1009+
"'operator=' should check for assignment to self to ensure that each block of dynamically "
1010+
"allocated memory is owned and managed by only one instance of the class.");
10071011
}
10081012

10091013
//---------------------------------------------------------------------------
@@ -1125,7 +1129,11 @@ void CheckClass::virtualDestructor()
11251129

11261130
void CheckClass::virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived)
11271131
{
1128-
reportError(tok, Severity::error, "virtualDestructor", "Class " + Base + " which is inherited by class " + Derived + " does not have a virtual destructor");
1132+
reportError(tok, Severity::error, "virtualDestructor", "Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor.\n"
1133+
"Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor. "
1134+
"If you destroy instances of the derived class by deleting a pointer that points to the base class, only "
1135+
"the destructor of the base class is executed. Thus, dynamic memory that is managed by the derived class "
1136+
"could leak. This can be avoided by adding a virtual destructor to the base class.");
11291137
}
11301138

11311139
//---------------------------------------------------------------------------
@@ -1152,7 +1160,7 @@ void CheckClass::thisSubtraction()
11521160

11531161
void CheckClass::thisSubtractionError(const Token *tok)
11541162
{
1155-
reportError(tok, Severity::warning, "thisSubtraction", "Suspicious pointer subtraction");
1163+
reportError(tok, Severity::warning, "thisSubtraction", "Suspicious pointer subtraction. Did you intend to write '->'?");
11561164
}
11571165

11581166
//---------------------------------------------------------------------------
@@ -1540,26 +1548,21 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func)
15401548

15411549
void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname)
15421550
{
1543-
reportError(tok, Severity::style, "functionConst",
1544-
"Technically the member function '" + classname + "::" + funcname + "' can be const.\n"
1545-
"The member function '" + classname + "::" + funcname + "' can be made a const "
1546-
"function. Making this function const function should not cause compiler errors. "
1547-
"Even though the function can be made const function technically it may not make "
1548-
"sense conceptually. Think about your design and task of the function first - is "
1549-
"it a function that must not change object internal state?", true);
1551+
checkConstError2(tok, 0, classname, funcname);
15501552
}
15511553

15521554
void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname)
15531555
{
15541556
std::list<const Token *> toks;
15551557
toks.push_back(tok1);
1556-
toks.push_back(tok2);
1558+
if (tok2)
1559+
toks.push_back(tok2);
15571560
reportError(toks, Severity::style, "functionConst",
15581561
"Technically the member function '" + classname + "::" + funcname + "' can be const.\n"
15591562
"The member function '" + classname + "::" + funcname + "' can be made a const "
1560-
"function. Making this function const function should not cause compiler errors. "
1563+
"function. Making this function 'const' should not cause compiler errors. "
15611564
"Even though the function can be made const function technically it may not make "
1562-
"sense conceptually. Think about your design and task of the function first - is "
1565+
"sense conceptually. Think about your design and the task of the function first - is "
15631566
"it a function that must not change object internal state?", true);
15641567
}
15651568

@@ -1644,8 +1647,10 @@ void CheckClass::initializerListError(const Token *tok1, const Token *tok2, cons
16441647
toks.push_back(tok2);
16451648
reportError(toks, Severity::style, "initializerList",
16461649
"Member variable '" + classname + "::" +
1647-
varname + "' is in the wrong order in the initializer list.\n"
1648-
"Members are initialized in the order they are declared, not the "
1650+
varname + "' is in the wrong place in the initializer list.\n"
1651+
"Member variable '" + classname + "::" +
1652+
varname + "' is in the wrong place in the initializer list. "
1653+
"Members are initialized in the order they are declared, not in the "
16491654
"order they are in the initializer list. Keeping the initializer list "
16501655
"in the same order that the members were declared prevents order dependent "
16511656
"initialization errors.", true);

‎lib/checkexceptionsafety.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,25 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
7575
private:
7676
/** Don't throw exceptions in destructors */
7777
void destructorsError(const Token * const tok) {
78-
reportError(tok, Severity::error, "exceptThrowInDestructor", "Throwing exception in destructor");
78+
reportError(tok, Severity::error, "exceptThrowInDestructor", "Exception thrown in destructor.");
7979
}
8080

8181
void deallocThrowError(const Token * const tok, const std::string &varname) {
82-
reportError(tok, Severity::warning, "exceptDeallocThrow", "Throwing exception in invalid state, " + varname + " points at deallocated memory");
82+
reportError(tok, Severity::warning, "exceptDeallocThrow", "Exception thrown in invalid state, '" +
83+
varname + "' points at deallocated memory.");
8384
}
8485

8586
void rethrowCopyError(const Token * const tok, const std::string &varname) {
8687
reportError(tok, Severity::style, "exceptRethrowCopy",
87-
"Throwing a copy of the caught exception instead of rethrowing the original exception\n"
88-
"Rethrowing an exception with 'throw " + varname + ";' makes an unnecessary copy of '" + varname + "'.\n"
88+
"Throwing a copy of the caught exception instead of rethrowing the original exception.\n"
89+
"Rethrowing an exception with 'throw " + varname + ";' creates an unnecessary copy of '" + varname + "'. "
8990
"To rethrow the caught exception without unnecessary copying or slicing, use a bare 'throw;'.");
9091
}
9192

9293
void catchExceptionByValueError(const Token *tok) {
9394
reportError(tok, Severity::style,
9495
"catchExceptionByValue", "Exception should be caught by reference.\n"
95-
"The exception is caught as a value. It could be caught "
96+
"The exception is caught by value. It could be caught "
9697
"as a (const) reference which is usually recommended in C++.");
9798
}
9899

@@ -116,7 +117,7 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
116117
"* Throwing exceptions in destructors\n"
117118
"* Throwing exception during invalid state\n"
118119
"* Throwing a copy of a caught exception instead of rethrowing the original exception\n"
119-
"* exception caught by value instead of by reference";
120+
"* Exception caught by value instead of by reference";
120121
}
121122
};
122123
/// @}

0 commit comments

Comments
 (0)