checking the qt 5 framework
DESCRIPTION
Static code analysis tools can help developers eliminate numbers of bugs as early as at the coding stage. With their help you can, for example, quickly catch and fix any typos. Well, some programmers are sincerely sure they never make typos and silly mistakes. But they are wrong; everyone makes mistakes. This article is a good evidence of that. Typos can be found even in high-quality and well tested projects such as Qt.TRANSCRIPT
CheckingtheQt5Framework
Author: Andrey Karpov
Date: 18.04.2014
Static code analysis tools can help developers eliminate numbers of bugs as early as at the coding stage.
With their help you can, for example, quickly catch and fix any typos. Well, some programmers are
sincerely sure they never make typos and silly mistakes. But they are wrong; everyone makes mistakes.
This article is a good evidence of that. Typos can be found even in high-quality and well tested projects
such as Qt.
Qt
Qt is a cross-platform application framework that is widely used for developing application software
with a graphical user interface (GUI) (in which cases Qt is classified as a widget toolkit), and also used for
developing non-GUI programs such as command-line tools and consoles for servers.
Qt uses standard C++ but makes extensive use of a special code generator (called the Meta Object
Compiler, or moc) together with several macros to enrich the language. Qt can also be used in several
other programming languages via language bindings. It runs on the major desktop platforms and some
of the mobile platforms. It has extensive internationalization support. Non-GUI features include SQL
database access, XML parsing, thread management, network support, and a unified cross-platform
application programming interface (API) for file handling. [the source: Wikipedia]
Links:
• Wikipedia: Qt;
• The official website: Qt Project;
• The article about Qt 4.7.3's check in 2011.
This time we were dealing with Qt 5.2.1. Analysis was done with the PVS-Studio 5.15 analyzer.
Please note that PVS-Studio managed to detect bugs despite that the Qt project had been checked
before by the Klocwork and Coverity analyzers. I don't know how regularly the project's authors use
these tools, but Klocwork and Coverity are mentioned in the bugtracker and ChangeLog-xxx files. I also
saw Qt mentioned to be regularly checked with PC-lint.
Specifics of the Qt project's analysis
Just for a change, we decided to check Qt using a new mechanism recently introduced in PVS-Studio
Standalone. Nobody knows of this mechanism yet, so we will remind you about it from time to time in
our next articles. Well, what is that mysterious and wonderful mechanism, after all?
In certain cases, you may have a difficult time trying to check a project with PVS-Studio - these are the
cases when the project is built with nmake and the like. You need to integrate PVS-Studio into the build,
which becomes not an easy thing to do. To say the least, quickly trying and making up an opinion about
the tool will become impossible.
But now PVS-Studio has acquired a new mode which makes it much simpler to work with such projects.
The analyzer has learned how to trace compilation parameters and collect all the necessary information
for analysis. You just need to tell the analyzer when to start monitoring compiler calls and when to stop
it.
Compilation monitoring can be controlled both from the GUI application and the command line. To find
out more about how it all works and how to use this mode, see the following article:
Evgeniy Ryzhkov. PVS-Studio Now Supports Any Build System under Windows and Any Compiler. Easy
and Right Out of the Box.
It describes the process of checking the Qt project with the monitoring mode launched from the
command line.
Do read it please to avoid any misconceptions. For example, you should keep in mind that you can't
code while project compilation is being monitored: if you compile files from another project, the
analyzer will collect the information about these files and check them too. It will result in the analysis
report including extraneous messages, the relevant and irrelevant warnings all mixed in one pile.
Analysis results
My general opinion of Qt's code is this:
It is pretty high-quality and is almost free of bugs related to dangerous specifics of the C++ language. On
the other hand, it does have quite a lot of ordinary typos.
This article is a good illustration of the thesis that every developer makes typos, however skillful he is.
Static code analysis has always been and will be topical and useful. Suppose the analyzer has found 10
typos with a one-time check. So it could have prevented hundreds or thousands of bugs by now if it had
been used regularly. That makes an enormous amount of time that could have been saved. Therefore it
is much more profitable to detect an error right after it has been made than at the stage of code
debugging or after user complaints.
Welcome to a wondrous world of typos
Typo No.1
bool QWindowsUser32DLL::initTouch()
{
QSystemLibrary library(QStringLiteral("user32"));
registerTouchWindow = ....;
unregisterTouchWindow = ....;
getTouchInputInfo = ....;
closeTouchInputHandle = ....;
return registerTouchWindow &&
unregisterTouchWindow &&
getTouchInputInfo &&
getTouchInputInfo;
}
PVS-Studio's diagnostic message: V501 There are identical sub-expressions 'getTouchInputInfo' to the
left and to the right of the '&&' operator. qwindowscontext.cpp 216
Values are assigned to four variables, and all the four must be checked. But only 3 are actually checked
because of a typo. In the last line, 'closeTouchInputHandle' should be written instead of
'getTouchInputInfo'.
Typo No.2
QWindowsNativeImage *QWindowsFontEngine::drawGDIGlyph(....)
{
....
int iw = gm.width.toInt();
int ih = gm.height.toInt();
if (iw <= 0 || iw <= 0)
return 0;
....
}
PVS-Studio's diagnostic message: V501 There are identical sub-expressions to the left and to the right of
the '||' operator: iw <= 0 || iw <= 0 qwindowsfontengine.cpp 1095
The check of the height parameter stored in the 'ih' variable is missing.
Typos No.3, No.4
This error was found inside tests. A nice example of how static analysis complements unit tests. To find
out more on this topic, see the article: "How to complement TDD with static analysis".
inline bool qCompare(QImage const &t1, QImage const &t2, ....)
{
....
if (t1.width() != t2.width() || t2.height() != t2.height()) {
....
}
PVS-Studio's diagnostic message: V501 There are identical sub-expressions to the left and to the right of
the '!=' operator: t2.height() != t2.height() qtest_gui.h 101
The function to compare two images is incorrectly comparing their heights. Or rather, it doesn't
compare them at all.
This bug was multiplied through the Copy-Paste method. The same comparison can be found a bit
farther in the code in the same file (line 135).
Typo No.5
I apologize for the ugly code formatting - the lines were too lengthy.
void QXmlSimpleReader::setFeature(
const QString& name, bool enable)
{
....
} else if ( name == QLatin1String(
"http://trolltech.com/xml/features/report-start-end-entity")
|| name == QLatin1String(
"http://trolltech.com/xml/features/report-start-end-entity"))
{
....
}
PVS-Studio's diagnostic message: V501 There are identical sub-expressions to the left and to the right of
the '||' operator. qxml.cpp 3249
The 'name' variable is compared to one and the same string twice. A bit earlier in the code, a similar
comparison can be found where a variable is compared with the following two strings:
• http://trolltech.com/xml/features/report-whitespace-only-CharData
• http://qt-project.org/xml/features/report-whitespace-only-CharData
By analogy, you can conclude that the 'name' variable in the fragment we are discussing should have
been compared with the following strings:
• http://trolltech.com/xml/features/report-start-end-entity
• http://qt-project.org/xml/features/report-start-end-entity
Typos No.6, No.7, No.8, No.9
QString DayTimeDuration::stringValue() const
{
....
if(!m_hours && !m_minutes && !m_seconds && !m_seconds)
....
}
PVS-Studio's diagnostic message: V501 There are identical sub-expressions '!m_seconds' to the left and
to the right of the '&&' operator. qdaytimeduration.cpp 148
The programmer forgot about milliseconds. Milliseconds are stored in the 'm_mseconds' variable. The
check should look like this:
if(!m_hours && !m_minutes && !m_seconds && !m_mseconds)
There are similar mistakes with milliseconds in three other fragments:
• qdaytimeduration.cpp 170
• qduration.cpp 167
• qduration.cpp 189
Typo No.10
QV4::ReturnedValue
QQuickJSContext2DPrototype::method_getImageData(
QV4::CallContext *ctx)
{
....
qreal x = ctx->callData->args[0].toNumber();
qreal y = ctx->callData->args[1].toNumber();
qreal w = ctx->callData->args[2].toNumber();
qreal h = ctx->callData->args[3].toNumber();
if (!qIsFinite(x) || !qIsFinite(y) ||
!qIsFinite(w) || !qIsFinite(w))
....
}
PVS-Studio's diagnostic message: V501 There are identical sub-expressions '!qIsFinite(w)' to the left and
to the right of the '||' operator. qquickcontext2d.cpp 3305
A check of the 'h' variable is missing. The 'w' variable is checked twice instead.
Typo No.11
AtomicComparator::ComparisonResult
IntegerComparator::compare(const Item &o1,
const AtomicComparator::Operator,
const Item &o2) const
{
const Numeric *const num1 = o1.as<Numeric>();
const Numeric *const num2 = o1.as<Numeric>();
if(num1->isSigned() || num2->isSigned())
....
}
V656 Variables 'num1', 'num2' are initialized through the call to the same function. It's probably an error
or un-optimized code. Consider inspecting the 'o1.as < Numeric > ()' expression. Check lines: 220, 221.
qatomiccomparators.cpp 221
The variables 'num1' and 'num2' are initialized to one and the same value. Then both the variables are
checked, and that is strange: it would be enough to check only one variable.
The 'num2' variable was most likely meant to be initialized to an expression with the 'o2' argument:
const Numeric *const num1 = o1.as<Numeric>();
const Numeric *const num2 = o2.as<Numeric>();
Typo No.12
void Atlas::uploadBgra(Texture *texture)
{
const QRect &r = texture->atlasSubRect();
QImage image = texture->image();
if (image.format() != QImage::Format_ARGB32_Premultiplied ||
image.format() != QImage::Format_RGB32) {
....
}
V547 Expression is always true. Probably the '&&' operator should be used here. qsgatlastexture.cpp
271
The condition in this code is meaningless as it is always true. Here you are a simplified sample to make it
clearer:
int a = ...;
if (a != 1 || a != 2)
The variable will always be not equal to something.
I can't say for sure what exactly the correct code should look like. It may be like this:
if (image.format() == QImage::Format_ARGB32_Premultiplied ||
image.format() == QImage::Format_RGB32) {
or this:
if (image.format() != QImage::Format_ARGB32_Premultiplied &&
image.format() != QImage::Format_RGB32) {
Typo No.13
void QDeclarativeStateGroupPrivate::setCurrentStateInternal(
const QString &state,
bool ignoreTrans)
{
....
QDeclarativeTransition *transition =
(ignoreTrans || ignoreTrans) ?
0 : findTransition(currentState, state);
....
}
PVS-Studio's diagnostic message: V501 There are identical sub-expressions to the left and to the right of
the '||' operator: ignoreTrans || ignoreTrans qdeclarativestategroup.cpp 442
Something is wrong with this code. I can't figure out how exactly the programmer meant to implement
the check.
Typo No.14
QV4::ReturnedValue
QQuickJSContext2DPrototype::method_createPattern(....)
{
....
if (repetition == QStringLiteral("repeat") ||
repetition.isEmpty()) {
pattern->patternRepeatX = true;
pattern->patternRepeatY = true;
} else if (repetition == QStringLiteral("repeat-x")) {
pattern->patternRepeatX = true;
} else if (repetition == QStringLiteral("repeat-y")) {
pattern->patternRepeatY = true;
} else if (repetition == QStringLiteral("no-repeat")) {
pattern->patternRepeatY = false;
pattern->patternRepeatY = false;
} else {
//TODO: exception: SYNTAX_ERR
}
....
}
PVS-Studio's diagnostic message: V519 The 'pattern->patternRepeatY' variable is assigned values twice
successively. Perhaps this is a mistake. Check lines: 1775, 1776. qquickcontext2d.cpp 1776
The 'patternRepeatY' variable is assigned values twice on end:
pattern->patternRepeatY = false;
pattern->patternRepeatY = false;
I guess the correct code should look as follows:
} else if (repetition == QStringLiteral("no-repeat")) {
pattern->patternRepeatX = false;
pattern->patternRepeatY = false;
} else {
Misuse of the C++ language
As I've already said, most bugs in this project are ordinary typos. There are almost no errors related to
misuse of the C++ language. However, the analyzer has caught a couple of these.
A nice error related to operation priorities
bool QConfFileSettingsPrivate::readIniLine(....)
{
....
char ch;
while (i < dataLen &&
((ch = data.at(i) != '\n') && ch != '\r'))
++i;
....
}
V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as
following: 'A = (B != C)'. qsettings.cpp 1702
The loop is designed to find the end of a string. The characters '\n' or '\r' are used as end-of-string
indicators.
Inside the condition, a character must be taken and compared against '\n' and '\r'. The error occurs
because the '!=' operator's priority is higher than that of the '=' operator. Because of this, the 'true' or
'false' value is written instead of the character code into the 'ch' variable. It makes the '\r' comparison
meaningless.
Let's arrange parentheses to make the error clearer:
while (i < dataLen &&
((ch = (data.at(i) != '\n')) && ch != '\r'))
Because of the mistake, it is only the '\n' character that is treated as an end-of-string indicator. The
function won't work correctly for strings ending with '\r'.
The fixed code should look as follows:
while (i < dataLen &&
(ch = data.at(i)) != '\n' && ch != '\r')
Loss of Accuracy
bool QWindowsTabletSupport::translateTabletPacketEvent()
{
....
const double radAzim =
(packet.pkOrientation.orAzimuth / 10) * (M_PI / 180);
....
}
V636 The 'packet.pkOrientation.orAzimuth / 10' expression was implicitly casted from 'int' type to
'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example:
double A = (double)(X) / Y;. qwindowstabletsupport.cpp 467
The 'packet.pkOrientation.orAzimuth' variable is of the 'int' type. This integer variable is divided by 10.
What is suspicious about this is that the quotient is then used together with values of the 'double' type.
The final result is also saved into a variable of the 'double' type.
Such integer division is not always an error. Perhaps this code is written just the way the programmer
intended. But practice shows that it is more often than not a mistake causing accuracy loss.
Suppose, for instance, that the 'packet.pkOrientation.orAzimuth' variable equals 55. Then the
calculation result will be:
(55 / 10) * (3.14159... / 180) = 5 * 0,01745... = 0,087266...
The accuracy of these calculations can be significantly improved by just declaring the 10 constant as of
the double type: "(packet.pkOrientation.orAzimuth / 10.0) * (M_PI / 180)". The result will then be:
(55 / 10.0) * (3.14159... / 180) = 5.5 * 0,01745... = 0,095993...
Accuracy losses like that often happen because of programmers being careless about expressions where
different types are used together. It is also due to this carelessness that many 64-bit errors occur (see
mixed arithmetic).
The analyzer has found 51 more suspicious cases of integer division. Some of them may prove less
accurate than the programmer wanted them to be. I've collected the corresponding diagnostic
messages in a separate list: qt-v636.txt.
Meaningless pointer checks
It has been for a long time now that checking a pointer for being null doesn't make any sense when the
'new' operator is used to allocate memory. Nowadays, it throws an exception when it fails to allocate
memory. Of course, you can make the 'new' operator return 0, but we are not speaking of these cases
now.
However, programmers sometimes forget about that and write meaningless checks in their code.
HRESULT STDMETHODCALLTYPE QWindowsEnumerate::Clone(
IEnumVARIANT **ppEnum)
{
QWindowsEnumerate *penum = 0;
*ppEnum = 0;
penum = new QWindowsEnumerate(array);
if (!penum)
return E_OUTOFMEMORY;
....
}
PVS-Studio's diagnostic message: V668 There is no sense in testing the 'penum' pointer against null, as
the memory was allocated using the 'new' operator. The exception will be generated in the case of
memory allocation error. qwindowsmsaaaccessible.cpp 141
There are some more checks like that in the project: main.cpp 127, qaudiodevicefactory.cpp 236,
qaudiodevicefactory.cpp 263, qaudiobuffer.cpp 488, mfvideorenderercontrol.cpp 143,
mfvideorenderercontrol.cpp 158, mfvideorenderercontrol.cpp 1193, mfvideorenderercontrol.cpp 1199,
qaxserverbase.cpp 1006, positionpollfactory.cpp 60.
The dark side
There are two code fragments in the Qt project about which I can't say for sure if they are errors or not
as I'm not familiar with the project architecture and its implementation specifics. But even if they don't
have errors, they certainly belong to the dark side of the C++ programming practice.
class Q_CORE_EXPORT QObject
{
....
virtual ~QObject();
virtual bool event(QEvent *);
virtual bool eventFilter(QObject *, QEvent *);
....
};
QObject *QQmlVME::run(....)
{
....
QObject *o = (QObject *)operator
new(instr.typeSize + sizeof(QQmlData));
::memset(static_cast<void *>(o), 0,
instr.typeSize + sizeof(QQmlData));
....
}
PVS-Studio's diagnostic message: V598 The 'memset' function is used to nullify the fields of 'QObject'
class. Virtual method table will be damaged by this. qqmlvme.cpp 658
The QObject class has virtual functions, which means the object stores a pointer to a virtual methods
table. I don't find it a good idea to implement such objects through the memset() function.
One more message of that kind: V598 The 'memset' function is used to nullify the fields of 'QObject'
class. Virtual method table will be damaged by this. qdeclarativevme.cpp 286
Null pointer dereferencing
I guess these errors could be classified as typos, but I like to single them out into a separate group. It
makes them look somewhat more somber and serious.
Note. Bug classification is pretty relative; many errors can be usually classified as a typo, a vulnerability,
an array overrun, and so on.
But let's get back to null pointers.
A typo leading to null pointer dereferencing
QV4::ReturnedValue QQuickJSContext2DPixelData::getIndexed(
QV4::Managed *m, uint index, bool *hasProperty)
{
....
if (!m)
return m->engine()->currentContext()->throwTypeError();
....
}
PVS-Studio's diagnostic message: V522 Dereferencing of the null pointer 'm' might take place.
qquickcontext2d.cpp 3169
I'm sure the '!' operator is unnecessary here. It's an ordinary typo leading to a serious bug.
Null pointer dereferencing in an error handler
void QDocIndexFiles::readIndexSection(....)
{
....
DocNode* dn = qdb_->findGroup(groupNames[i]);
if (dn) {
dn->addMember(node);
}
else {
....
qDebug() << "DID NOT FIND GROUP:" << dn->name()
<< "for:" << node->name();
}
....
}
PVS-Studio's diagnostic message: V522 Dereferencing of the null pointer 'dn' might take place.
qdocindexfiles.cpp 539
If an error occurs, the program must print an error message trying to take the name from a nonexistent
object: dn->name().
82 potential null pointer dereferencing errors
Most project (and Qt is no exception) have null pointer handling issues. The check is often done after
the pointer has been used. It's not always an error; there are cases when the pointer just can never be
null.
But anyway, such fragments need to be attentively checked and refactored. Even if there is no error, an
excess pointer check confuses the programmer reading the code.
Have a look at one dangerous code sample:
static int gray_raster_render(....)
{
const QT_FT_Outline* outline =
(const QT_FT_Outline*)params->source;
....
/* return immediately if the outline is empty */
if ( outline->n_points == 0 || outline->n_contours <= 0 )
return 0;
if ( !outline || !outline->contours || !outline->points )
return ErrRaster_Invalid_Outline;
....
}
PVS-Studio's diagnostic message: V595 The 'outline' pointer was utilized before it was verified against
nullptr. Check lines: 1746, 1749. qgrayraster.c 1746
I guess the error must have appeared when the programmer was trying to optimize the
gray_raster_render() function. It seems that the following lines were added later into an already
complete function code:
/* return immediately if the outline is empty */
if ( outline->n_points == 0 || outline->n_contours <= 0 )
return 0;
The trouble is that the 'outline' pointer may be null, but the necessary check is written after that
fragment.
The analyzer has found 81 more potential issues like that. Here is a complete list of them: qt-v595.txt.
Questions without answers
There are strange code fragments about whose origin and the programmer's intentions about them you
can't be sure. They may be typos or incomplete code or unsuccessful refactoring - whatever.
Double check
QWindowsFontEngine::~QWindowsFontEngine()
{
....
if (QWindowsContext::verboseFonts)
if (QWindowsContext::verboseFonts)
qDebug("%s: font='%s", __FUNCTION__, qPrintable(_name));
....
}
PVS-Studio's diagnostic message: V571 Recurring check. The 'if (QWindowsContext::verboseFonts)'
condition was already verified in line 369. qwindowsfontengine.cpp 370
What's the use checking one and the same thing twice? One of the checks is probably excess; or
something else was meant to be checked.
Double assignment
void Moc::parse()
{
....
index = def.begin + 1;
namespaceList += def;
index = rewind;
....
}
PVS-Studio's diagnostic message: V519 The 'index' variable is assigned values twice successively.
Perhaps this is a mistake. Check lines: 568, 570. moc.cpp 570
Why different values are assigned to the 'index' variable?
There are a few more similar strange code fragments:
• V519 The 'exitCode' variable is assigned values twice successively. Perhaps this is a mistake.
Check lines: 807, 815. qprocess.cpp 815
• V519 The 'detecting' variable is assigned values twice successively. Perhaps this is a mistake.
Check lines: 163, 164. qhoversensorgesturerecognizer.cpp 164
• V519 The 'increaseCount' variable is assigned values twice successively. Perhaps this is a
mistake. Check lines: 185, 186. qtwistsensorgesturerecognizer.cpp 186
Suspecting a missing 'break' operator
bool GSuggestCompletion::eventFilter(QObject *obj, QEvent *ev)
{
....
switch (key) {
case Qt::Key_Enter:
case Qt::Key_Return:
doneCompletion();
consumed = true;
case Qt::Key_Escape:
editor->setFocus();
popup->hide();
consumed = true;
case Qt::Key_Up:
case Qt::Key_Down:
case Qt::Key_Home:
case Qt::Key_End:
case Qt::Key_PageUp:
case Qt::Key_PageDown:
break;
....
}
PVS-Studio's diagnostic message: V519 The 'consumed' variable is assigned values twice successively.
Perhaps this is a mistake. Check lines: 110, 115. googlesuggest.cpp 115
So is the break operator missing here or not?
The analyzer found it strange that the 'consumed' variable was assigned the 'true' value twice on end. It
suggests a missing break operator, but I'm not sure. It may be just that the first assignment should be
removed: "consumed = true;".
Suspecting an excess 'break' operator
bool QHelpGenerator::registerVirtualFolder(....)
{
....
while (d->query->next()) {
d->namespaceId = d->query->value(0).toInt();
break;
}
....
}
PVS-Studio's diagnostic message: V612 An unconditional 'break' within a loop. qhelpgenerator.cpp 429
Was the 'break' operator really meant to terminate the loop right away?
One more fragment of that kind can be found here: qhelpgenerator.cpp 642
Miscellaneous
Be patient: there's not much left, just a handful of diverse errors.
Incorrect use of the toLower() function
int main(int argc, char **argv)
{
....
QByteArray arg(argv[a]);
....
arg = arg.mid(1);
arg.toLower();
if (arg == "o")
....
}
PVS-Studio's diagnostic message: V530 The return value of function 'toLower' is required to be utilized.
main.cpp 72
The 'toLower()' function doesn't change the object - it returns a copy of an object that will store lower
case characters.
One more defect: V530 The return value of function 'toLower' is required to be utilized. main.cpp 1522
Array index out of bounds
It's a complicated issue, so please be attentive.
There is an enum type in the code:
typedef enum {
JNone,
JCausing,
JDual,
JRight,
JTransparent
} Joining;
Note that JTransparent == 4 and keep that in mind.
Now let's examine the getNkoJoining() function:
static Joining getNkoJoining(unsigned short uc)
{
if (uc < 0x7ca)
return JNone;
if (uc <= 0x7ea)
return JDual;
if (uc <= 0x7f3)
return JTransparent;
if (uc <= 0x7f9)
return JNone;
if (uc == 0x7fa)
return JCausing;
return JNone;
}
What matters to us is that this function may return 'JTransparent', i.e. the function may return 4.
There is also a two-dimensional array 'joining_table':
static const JoiningPair joining_table[5][4] = { .... };
And here's the piece of code itself where the error may occur:
static void getNkoProperties(....)
{
....
Joining j = getNkoJoining(chars[0]);
ArabicShape shape = joining_table[XIsolated][j].form2;
....
}
PVS-Studio's diagnostic message: V557 Array overrun is possible. The value of 'j' index could reach 4.
harfbuzz-arabic.c 516
As we remember, the getNkoJoining() function may return 4. Thus, we will be addressing the array cell
joining_table[...][4] in this case, which is illegal because an array overrun will occur.
Identical conditions
void Node::setPageType(const QString& t)
{
if ((t == "API") || (t == "api"))
pageType_ = ApiPage;
else if (t == "howto")
pageType_ = HowToPage;
else if (t == "overview")
pageType_ = OverviewPage;
else if (t == "tutorial")
pageType_ = TutorialPage;
else if (t == "howto")
pageType_ = HowToPage;
else if (t == "article")
pageType_ = ArticlePage;
else if (t == "example")
pageType_ = ExamplePage;
else if (t == "ditamap")
pageType_ = DitaMapPage;
}
PVS-Studio's diagnostic message: V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There
is a probability of logical error presence. Check lines: 386, 392. node.cpp 386
The (t == "howto") check is executed twice. I guess one of the checks is not necessary.
Here are a couple of other similar warnings:
• V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical
error presence. Check lines: 188, 195. qmaintainingreader_tpl_p.h 188
• V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical
error presence. Check lines: 299, 303. mfmetadatacontrol.cpp 299
Identical branches are executed
void
QBluetoothServiceDiscoveryAgentPrivate::_q_deviceDiscovered(
const QBluetoothDeviceInfo &info)
{
if(mode == QBluetoothServiceDiscoveryAgent::FullDiscovery) {
for(int i = 0; i < discoveredDevices.count(); i++){
if(discoveredDevices.at(i).address() == info.address()){
discoveredDevices.removeAt(i);
}
}
discoveredDevices.prepend(info);
}
else {
for(int i = 0; i < discoveredDevices.count(); i++){
if(discoveredDevices.at(i).address() == info.address()){
discoveredDevices.removeAt(i);
}
}
discoveredDevices.prepend(info);
}
}
PVS-Studio's diagnostic message: V523 The 'then' statement is equivalent to the 'else' statement.
qbluetoothservicediscoveryagent.cpp 402
Regardless of the condition, one and the same code branch is executed.
Other similar defects: pcre_exec.c 5577, ditaxmlgenerator.cpp 1722, htmlgenerator.cpp 388.
Inherited errors
Qt employs a few third-party libraries. Those also contain errors, therefore they can be said to belong to
Qt as well. I decided not to describe them in the article, but I should mention them at least.
I didn't study the reports for the libraries attentively, but I have noted down some bugs: qt-3rdparty.txt.
Note. Don't assume, however, that I was attentively studying bugs from Qt instead. The project is pretty
large and even a superficial analysis was enough to collect examples for this article.
Conclusions
PVS-Studio is an excellent, powerful analyzer capable of catching bugs even in high-quality and cleaned
up projects such as the Qt framework.
It can help a developer team save huge amounts of time by revealing many bugs at the earliest
development stage. With the incremental analysis mode enabled, errors will be detected immediately
after compilation.
References
1. We regularly check open-source projects. For example: Tor, Chromium, Clang, Firebird, OpenCV.
All those interested are welcome: "Updatable List of Open-Source Projects Checked with PVS-
Studio".
2. Here you can download the PVS-Studio trial version. For a start you are given 20 clicks to
navigate through the diagnostic messages. After submitting your personal information, you are
granted 200 more clicks.
3. Things are much simpler with the lightweight CppCat analyzer. This tool is an excellent solution
for small teams and single developers. The trial version is fully functional for 7 days. The license
costs $250 and can be renewed at $200. We also offer discounts for purchasing several copies at
once.