Published in


A Third Check of Qt 5 with PVS-Studio

Every now and then, we re-check projects that we already checked and mentioned in our articles in the past. Qt is one of them. The last time we checked it with PVS-Studio was in 2014. Starting with 2014, the project has been regularly checked with Coverity, which makes things more interesting. Let’s see if PVS-Studio can find any cool bugs this time.


The previous articles:

This time we checked Qt Base (Core, Gui, Widgets, Network, …) and Qt5 super module. As for Qt Creator, we plan on writing a separate article about it later. The check was done with the PVS-Studio static analyzer; you can download the demo version from our website.

Personally, I think that the code quality of Qt has improved. During the years since the last check, we have added lots of new diagnostics to PVS-Studio. Despite that, a quick review of the analyzer warnings showed that there were relatively few bugs for a project that size. As I said, that’s my own impression of the code; I didn’t do any special research on the error density, either earlier or now.

It seems the high code quality results from the regular checks with the Coverity static analyzer. Starting with 2014, the developers have been using it to check their project (qt-project), and starting with 2016, Qt Creator (qt-creator). In my opinion, if you develop an open-source project, Coverity Scan is a good choice among free tools and can greatly improve the quality and reliability of your projects.

Anyway, I obviously wouldn’t have written this article if I hadn’t found anything worthy in the PVS-Studio report :). And since it’s here, it means I found some bugs. Let’s see what we’ve got here. The total number of defects I noted down is 96.

Bad copy-paste and typos

Let’s start with a classic: errors resulting from inattention. Such errors are often underestimated, and if you haven’t yet read these two articles, I recommend doing so:

Errors of this type are common in every language. For example, the second article above shows lots of bug examples in comparison functions written in C, C++, and C#. Now, as we work on adding Java support to PVS-Studio, we are seeing the same error patterns. Here’s, for instance, an error we recently found in the Hibernatelibrary:

public boolean equals(Object other) {
if (other instanceof Id) {
Id that = (Id) other;
return purchaseSequence.equals(this.purchaseSequence) &&
that.purchaseNumber == this.purchaseNumber;
else {
return false;

If you look closely, you’ll notice that the purchaseSequence field is compared with itself. This is the correct version:

return that.purchaseSequence.equals(this.purchaseSequence) &&
that.purchaseNumber == this.purchaseNumber;

It’s the same old story, and now PVS-Studio will have to “clean the Augean stables” inside Java projects as well. By the way, everyone interested is welcome to take part in the beta testing of PVS-Studio for Java, which is due to release soon. E-mail us if you want to participate (select the subject “I want to analyze Java”).

Going back to the bugs in Qt.

Defect 1

static inline int windowDpiAwareness(HWND hwnd)
return QWindowsContext::user32dll.getWindowDpiAwarenessContext &&
? QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext(
: -1;

PVS-Studio diagnostic message: V501 CWE-571 There are identical sub-expressions ‘QWindowsContext::user32dll.getWindowDpiAwarenessContext’ to the left and to the right of the ‘&&’ operator. qwindowscontext.cpp 150

This case doesn’t need any special comment besides the message. I think the expression was meant to look like this:

return QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext &&
? QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext(
: -1;

Defects 2, 3

void QReadWriteLockPrivate::release()
Q_ASSERT(!waitingReaders && !waitingReaders &&
!readerCount && !writerCount);

PVS-Studio diagnostic message: V501 CWE-571 There are identical sub-expressions to the left and to the right of the ‘&&’ operator: !waitingReaders &&!waitingReaders qreadwritelock.cpp 632

The error is inside the condition in the Q_ASSERT macro, which makes it a minor defect, but it’s still an error. The waitingReaders variable is checked twice, which means some other variable wasn’t checked at all.

The same bug is found in line 625 of the qreadwritelock.cpp file. Hail to copy-paste! :)

Defect 4

QString QGraphicsSceneBspTree::debug(int index) const
if (node->type == Node::Horizontal) {
tmp += debug(firstChildIndex(index));
tmp += debug(firstChildIndex(index) + 1);
} else {
tmp += debug(firstChildIndex(index));
tmp += debug(firstChildIndex(index) + 1);

PVS-Studio diagnostic message: V523 CWE-691 The ‘then’ statement is equivalent to the ‘else’ statement. qgraphicsscene_bsp.cpp 179

It looks like the programmer copied this block of code but forgot to modify it.

Defect 5

enum FillRule {
QDataStream &operator>>(QDataStream &s, QPainterPath &p)
int fillRule;
s >> fillRule;
Q_ASSERT(fillRule == Qt::OddEvenFill || Qt::WindingFill);

PVS-Studio diagnostic message: V768 CWE-571 The enumeration constant ‘WindingFill’ is used as a variable of a Boolean-type. qpainterpath.cpp 2479

This one is so cool! Q_ASSERT doesn’t check anything as the condition is always true. And it’s true because the value of the named constant Qt::WindingFill is 1.

Defect 6

bool QVariant::canConvert(int targetTypeId) const
if (currentType == QMetaType::SChar || currentType == QMetaType::Char)
currentType = QMetaType::UInt;
if (targetTypeId == QMetaType::SChar || currentType == QMetaType::Char)
targetTypeId = QMetaType::UInt;

Try to find the bug on your own before going on to read the warning. To make sure you don’t look at it right away, here’s a nice picture :).

PVS-Studio diagnostic message: V560 CWE-570 A part of conditional expression is always false: currentType == QMetaType::Char. qvariant.cpp 3529

The “currentType == QMetaType::Char” condition is checked in the first ifstatement. If it is true, the currentType variable is assigned the value QMetaType::UInt. It means there’s no way it could become equal to QMetaType::Char after that. That’s why the analyzer is telling us that the “currentType == QMetaType::Char” subexpression in the second if statement is always false.

The second if should actually look like this:

if (targetTypeId == QMetaType::SChar || targetTypeId == QMetaType::Char)
targetTypeId = QMetaType::UInt;

Note on diagnostic V560

There were lots of V560 warnings in the report, but I stopped reading when I found an interesting case to include in the article — see Defect 6 above.

Most of the V560 warnings can’t be called false positives but they are still of no use. In other words, they aren’t interesting to discuss. Here’s one example to explain what I mean.

QString QTextHtmlExporter::findUrlForImage(const QTextDocument *doc, ....)
QString url;
if (!doc)
return url;
if (QTextDocument *parent = qobject_cast<QTextDocument *>(doc->parent()))
return findUrlForImage(parent, cacheKey, isPixmap);

if (doc && doc->docHandle()) { // <=

PVS-Studio diagnostic message: V560 CWE-571 A part of conditional expression is always true: doc. qtextdocument.cpp 2992

The analyzer is totally correct when saying that the doc pointer is always not equal to nullptr in the second check. But it’s not a bug; the programmer was just playing safe. The code can be simplified as follows:

if (doc->docHandle()) {

Defect 7

This is the last case we could classify as a typo. The error is the result of confusing the constants’ names, which differ only in the case of the first letter.

class QWindowsCursor : public QPlatformCursor
enum CursorState {
QWindowsCursor::CursorState QWindowsCursor::cursorState()
enum { cursorShowing = 0x1, cursorSuppressed = 0x2 };
CURSORINFO cursorInfo;
cursorInfo.cbSize = sizeof(CURSORINFO);
if (GetCursorInfo(&cursorInfo)) {
if (cursorInfo.flags & CursorShowing)

PVS-Studio diagnostic message: V616 CWE-480 The ‘CursorShowing’ named constant with the value of 0 is used in the bitwise operation. qwindowscursor.cpp 669

I already discussed this defect in detail in a small separate post some time ago: “Once again the PVS-Studio analyzer has proved to be more attentive than a person”.

Security issues

To tell the truth, all the bugs discussed here could be called security issues; they all fall into the Common Weakness Enumeration classification (see the CWE ID tag in the analyzer warnings). Errors registered in the CWE are potentially dangerous from the security viewpoint. For more details on this topic, see the page PVS-Studio SAST.

However, some of the bugs call for being put in a separate group. Let’s take a look at them.

Defects 8, 9

bool QLocalServerPrivate::addListener()
SetSecurityDescriptorOwner(pSD.data(), pTokenUser->User.Sid, FALSE);
SetSecurityDescriptorGroup(pSD.data(), pTokenGroup->PrimaryGroup, FALSE);

PVS-Studio diagnostic messages:

  • V530 CWE-252 The return value of function ‘SetSecurityDescriptorOwner’ is required to be utilized. qlocalserver_win.cpp 167
  • V530 CWE-252 The return value of function ‘SetSecurityDescriptorGroup’ is required to be utilized. qlocalserver_win.cpp 168

There are functions that deal with access control, and the functions SetSecurityDescriptorOwner and SetSecurityDescriptorGroup are among them.

You should be very cautious when using these functions. Always check on the status they return. What if a call to them fails? Don’t make guesses, just write code to handle that case.

Missing checks aren’t necessarily easy to exploit to turn defects like that into vulnerabilities. But you still don’t want to take the risk. Write safer code.

Defect 10

bool QLocalServerPrivate::addListener()
InitializeAcl(acl, aclSize, ACL_REVISION_DS);

PVS-Studio diagnostic message: V530 CWE-252 The return value of function ‘InitializeAcl’ is required to be utilized. qlocalserver_win.cpp 144

This one is similar to the previous case.

Defects 11, 12

static inline void sha1ProcessChunk(....)
quint8 chunkBuffer[64];
memset(chunkBuffer, 0, 64);

PVS-Studio diagnostic message: V597 CWE-14 The compiler could delete the ‘memset’ function call, which is used to flush ‘chunkBuffer’ buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sha1.cpp 189

The compiler will delete the call to the memset function. I already discussed this situation in many posts before and don’t want to repeat myself. See this article: “Safe Clearing of Private Data”.

Another vulnerability is found in the same file sha1.cpp, line 247.

Null pointers

Now it’s time to talk about pointers. There are quite a lot of errors from this group.

Defect 13

QByteArray &QByteArray::append(const char *str, int len)
if (len < 0)
len = qstrlen(str);
if (str && len) {

PVS-Studio diagnostic message: V595 CWE-476 The ‘str’ pointer was utilized before it was verified against nullptr. Check lines: 2118, 2119. qbytearray.cpp 2118

This is a typical situation: a pointer is first used and then checked against nullptr. This error pattern is very common, and we see it all the time in almost every project.

Defects 14, 15

static inline const QMetaObjectPrivate *priv(const uint* data)
{ return reinterpret_cast<const QMetaObjectPrivate*>(data); }
bool QMetaEnum::isFlag() const
const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1;
return mobj && mobj->d.data[handle + offset] & EnumIsFlag;

PVS-Studio diagnostic message: V595 CWE-476 The ‘mobj’ pointer was utilized before it was verified against nullptr. Check lines: 2671, 2672. qmetaobject.cpp 2671

I’m citing the body of the priv function just in case. For some reason, readers will sometimes come up with scenarios where they believe this code would work. I wonder where this mistrust and desire to view a bug as a tricky feature come from :). For instance, somebody could write a comment suggesting that priv is a macro of this pattern:

#define priv(A) foo(sizeof(A))

In this case, they say, everything will be fine.

To avert any such debates, I try to give all the necessary information when citing code snippets to prove that the code is indeed faulty.

So, the modj pointer is dereferenced and then checked.

Then the “Great and Powerful” copy-paste comes into play, creating a clone of this bug in the isScoped function:

bool QMetaEnum::isScoped() const
const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1;
return mobj && mobj->d.data[handle + offset] & EnumIsScoped;

PVS-Studio diagnostic message: V595 CWE-476 The ‘mobj’ pointer was utilized before it was verified against nullptr. Check lines: 2683, 2684. qmetaobject.cpp 2683

Defects 16–21

The last example from this group.

void QTextCursor::insertFragment(const QTextDocumentFragment &fragment)
if (!d || !d->priv || fragment.isEmpty())
if (fragment.d && fragment.d->doc)

PVS-Studio diagnostic message: V595 CWE-476 The ‘fragment.d’ pointer was utilized before it was verified against nullptr. Check lines: 2238, 2241. qtextcursor.cpp 2238

Nothing new here. Note the order of operations performed on the pointer stored in the fragment.d variable.

Other bugs of this type:

  • V595 CWE-476 The ‘window’ pointer was utilized before it was verified against nullptr. Check lines: 1846, 1848. qapplication.cpp 1846
  • V595 CWE-476 The ‘window’ pointer was utilized before it was verified against nullptr. Check lines: 1858, 1860. qapplication.cpp 1858
  • V595 CWE-476 The ‘reply’ pointer was utilized before it was verified against nullptr. Check lines: 492, 502. qhttpnetworkconnectionchannel.cpp 492
  • V595 CWE-476 The ‘newHandle’ pointer was utilized before it was verified against nullptr. Check lines: 877, 883. qsplitter.cpp 877
  • V595 CWE-476 The ‘widget’ pointer was utilized before it was verified against nullptr. Check lines: 2320, 2322. qwindowsvistastyle.cpp 2320
  • This list is actually longer, but I quickly got tired of looking through the V595 warnings, and I had enough example snippets for the article.

Defects 22–33

Some checks test the pointer returned by the new operator. This is especially funny considering that there are lots of cases where the return result of the mallocfunction is not checked at all (see the next group of defects).

bool QTranslatorPrivate::do_load(const QString &realname,
const QString &directory)
d->unmapPointer = new char[d->unmapLength];
if (d->unmapPointer) {
qint64 readResult = file.read(d->unmapPointer, d->unmapLength);
if (readResult == qint64(unmapLength))
ok = true;

PVS-Studio diagnostic message: V668 CWE-571 There is no sense in testing the ‘d->unmapPointer’ pointer against null, as the memory was allocated using the ‘new’ operator. The exception will be generated in the case of memory allocation error. qtranslator.cpp 596

The pointer check makes no sense because memory allocation failure will raise an std::bad_alloc exception. If the developers wanted the new operator to return nullptr when memory can’t be allocated, they should have written it as follows:

d->unmapPointer = new (std::nothrow) char[d->unmapLength];

The analyzer knows about this implementation of the new operator and doesn’t issue warnings on it.

Other defects: see the file qt-V668.txt.

Defects 34–70

As I promised, here’s a group of errors that have to do with missing checks of the values returned by the functions malloc, calloc, strdup, etc. These are more severe than you might think. See the article “Why it is important to check what the malloc function returned” for details.

nodes = (SourceFileNode**)malloc(sizeof(SourceFileNode*)*(num_nodes=3037));
for(int n = 0; n < num_nodes; n++)
nodes[n] = nullptr;

PVS-Studio diagnostic message: V522 CWE-690 There might be dereferencing of a potential null pointer ‘nodes’. Check lines: 138, 136. makefiledeps.cpp 138

The pointer is used without a prior check.

All these defects are alike, so I won’t go into much detail. The rest of warnings of this type are listed in qt-V522-V575.txt.

Logical errors in conditions

Defect 71

QString QEdidParser::parseEdidString(const quint8 *data)
QByteArray buffer(reinterpret_cast<const char *>(data), 13);
// Erase carriage return and line feed
buffer = buffer.replace('\r', '\0').replace('\n', '\0');
// Replace non-printable characters with dash
for (int i = 0; i < buffer.count(); ++i) {
if (buffer[i] < '\040' && buffer[i] > '\176')
buffer[i] = '-';
return QString::fromLatin1(buffer.trimmed());

PVS-Studio diagnostic message: V547 CWE-570 Expression ‘buffer[i] < ‘\040’ && buffer[i] > ‘\176’’ is always false. qedidparser.cpp 169

The function is meant to “Replace non-printable characters with dash”, which it doesn’t. Let’s look into the following condition:

if (buffer[i] < '\040' && buffer[i] > '\176')

It makes no sense. A character cannot be less than ‘\040’ and greater than ‘\176’ at the same time. The ‘||’ operator should be used instead:

if (buffer[i] < '\040' || buffer[i] > '\176')

Defect 72

Another similar bug, which will affect Windows users.

#if defined(Q_OS_WIN)
static QString driveSpec(const QString &path)
if (path.size() < 2)
return QString();
char c = path.at(0).toLatin1();
if (c < 'a' && c > 'z' && c < 'A' && c > 'Z')
return QString();
if (path.at(1).toLatin1() != ':')
return QString();
return path.mid(0, 2);

This code triggers two warnings at once:

  • V590 CWE-571 Consider inspecting the ‘c < ‘a’ && c > ‘z’ && c < ‘A’ && c > ‘Z’’ expression. The expression is excessive or contains a misprint. qdir.cpp 77
  • V560 CWE-570 A part of conditional expression is always false: c > ‘z’. qdir.cpp 77

The logical error is found in the following condition:

if (c < 'a' && c > 'z' && c < 'A' && c > 'Z')

As far as I understand, the developer’s intention was to find a character that didn’t belong to the Latin alphabet. If so, the condition should look like this:

if ((c < 'a' || c > 'z') && (c < 'A' || c > 'Z'))

Defect 73

enum SelectionMode {
void QAccessibleTableCell::unselectCell()
QAbstractItemView::SelectionMode selectionMode = view->selectionMode();
if (!m_index.isValid() || (selectionMode & QAbstractItemView::NoSelection))

PVS-Studio diagnostic message: V616 CWE-480 The ‘QAbstractItemView::NoSelection’ named constant with the value of 0 is used in the bitwise operation. itemviews.cpp 976

Since the value of the named constant QAbstractItemView::NoSelection is zero, the (selectionMode & QAbstractItemView::NoSelection) subexpression is meaningless: it will always evaluate to 0.

I think the authors intended to write the following:

if (!m_index.isValid() || (selectionMode == QAbstractItemView::NoSelection))

Defect 74

The snippet below is something that I can’t fully figure out. It’s faulty, but I’m still not sure what its correct version should look like. The comment doesn’t help either.

// Re-engineered from the inline function _com_error::ErrorMessage().
// We cannot use it directly since it uses swprintf_s(), which is not
// present in the MSVCRT.DLL found on Windows XP (QTBUG-35617).
static inline QString errorMessageFromComError(const _com_error &comError)
TCHAR *message = nullptr;
message, 0, NULL);
if (message) {
const QString result = QString::fromWCharArray(message).trimmed();
return result;
if (const WORD wCode = comError.WCode())
return QString::asprintf("IDispatch error #%u", uint(wCode));
return QString::asprintf("Unknown error 0x0%x", uint(comError.Error()));

PVS-Studio diagnostic message: V547 CWE-570 Expression ‘message’ is always false. qwindowscontext.cpp 802

The programmer was probably assuming that the FormatMessage function would change the value of the message pointer, but that’s wrong. The FormatMessagefunction can’t do that because the pointer is passed by value. Here’s the function’s prototype:

DWORD dwFlags,
LPCVOID lpSource,
DWORD dwMessageId,
DWORD dwLanguageId,
LPWSTR lpBuffer,
DWORD nSize,
va_list *Arguments

Potential memory leaks

Defects 75–92

struct SourceDependChildren {
SourceFile **children;
int num_nodes, used_nodes;
SourceDependChildren() : children(nullptr), num_nodes(0), used_nodes(0) { }
~SourceDependChildren() { if (children) free(children); children = nullptr; }
void addChild(SourceFile *s) {
if(num_nodes <= used_nodes) {
num_nodes += 200;
children = (SourceFile**)realloc(children,
children[used_nodes++] = s;

PVS-Studio diagnostic message: V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer ‘children’ is lost. Consider assigning realloc() to a temporary pointer. makefiledeps.cpp 103

The buffer resizing is done in a dangerous way. If the realloc function fails to allocate the memory block, it will return a NULL value, which will be immediately assigned to the children variable, leaving no chance to free the previously allocated buffer. The program ends up with a memory leak.

Other similar defects: qt-701.txt.


Defect 93

template<class GradientBase, typename BlendType>
static inline const BlendType * QT_FASTCALL
if (t+inc*length < qreal(INT_MAX >> (FIXPT_BITS + 1)) &&
t+inc*length > qreal(INT_MIN >> (FIXPT_BITS + 1))) {

PVS-Studio diagnostic message: V610 CWE-758 Unspecified behavior. Check the shift operator ‘>>’. The left operand ‘(- 2147483647–1)’ is negative. qdrawhelper.cpp 4015

You can’t shift a negative INT_MIN value. This is unspecified behavior, and you can’t rely on the result of such an operation. The most significant bits may happen to contain 0s as well as 1s.

Defect 94

void QObjectPrivate::addConnection(int signal, Connection *c)
if (signal >= connectionLists->count())
connectionLists->resize(signal + 1);
ConnectionList &connectionList = (*connectionLists)[signal];
if (signal < 0) {

PVS-Studio diagnostic message: V781 CWE-129 The value of the ‘signal’ variable is checked after it was used. Perhaps there is a mistake in program logic. Check lines: 397, 413. qobject.cpp 397

The check (signal < 0) suggests that the value of the signal argument might be negative. But this argument was used earlier to index into an array. It means execution will reach the check too late; the program will have been affected by then.

Defect 95

bool QXmlStreamWriterPrivate::finishStartElement(bool contents)
if (inEmptyElement) {
QXmlStreamWriterPrivate::Tag &tag = tagStack_pop();
lastNamespaceDeclaration = tag.namespaceDeclarationsSize;
lastWasStartElement = false;
} else {
inStartElement = inEmptyElement = false;
lastNamespaceDeclaration = namespaceDeclarations.size();
return hadSomethingWritten;

PVS-Studio diagnostic message: V519 CWE-563 The ‘lastNamespaceDeclaration’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3188, 3194. qxmlstream.cpp 3194

Here’s the most important part:

if (inEmptyElement) {
lastNamespaceDeclaration = tag.namespaceDeclarationsSize;
lastNamespaceDeclaration = namespaceDeclarations.size();

Defect 96

void QRollEffect::scroll()
if (currentHeight != totalHeight) {
currentHeight = totalHeight * (elapsed/duration)
+ (2 * totalHeight * (elapsed%duration) + duration)
/ (2 * duration);
// equiv. to int((totalHeight*elapsed) / duration + 0.5)
done = (currentHeight >= totalHeight);
done = (currentHeight >= totalHeight) &&
(currentWidth >= totalWidth);

V519 CWE-563 The ‘done’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 509, 511. qeffects.cpp 511

Just the same as in the previous case. Note the done variable.


Even a quick review revealed about 100 defects. I’m glad about how PVS-Studio performed.

Of course, occasional checks like that have nothing in common with improving code quality and reliability. They are just good enough to demonstrate what the analyzer is capable of. Static analysis tools must be used regularly: only then will they help reduce the price of bug fixing and protect your programs from many potential vulnerabilities.

Thanks for reading. Stay tuned by following our channels:




PVS-Studio is a tool for detecting bugs and security weaknesses in the source code of programs, written in C, C++, C# and Java. It works under 64-bit systems in Windows, Linux and macOS environments, and can analyze source code intended for 32-bit, 64-bit and embedded ARM platfor

Recommended from Medium

Securing REST API with Spring Security, JWT, and JPA

How to Reduce PDF File Size in C/C++

A note on the latest features integrated with Epixel MLM Software!

Check PostgreSQL INSERT in real-time

Ansible Playbook for installing Azure Agent on VM

Parsing wikipedia in 2020

Creating Immersion with Sound

FreeBSD Desktop - Part 9 - Key Components - Keyboard/Mouse Shortcuts

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Unicorn Developer

Unicorn Developer

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.

More from Medium

Should PVS-Studio process other tools’ reports?

Fundamentals of functional programming

C++20 Concurrency: part-3 request_stop and stop_token for std::jthread