Unicorn interested in KDE

KDE (short for K Desktop Environment) is a desktop environment, mainly for Linux and other UNIX-like systems. If in simple terms, this is the thing that is responsible for all graphic design. The environment is based on the Qt cross-platform user interface development toolkit. The development involved several hundred programmers from around the world, committed to the idea of ​​free software. KDE offers a complete set of user environment applications that allow you to interact with the operating system in a modern graphical interface. Let's see what KDE has under the hood.

The following packages of the KDE project version 4.14 were checked with PVS-Studio 5.19 in the OpenSUSE Factory:

• KDE PIM Libraries
• KDE Base Libraries
• KDE Base Apps
• KDE Development

KDE PIM Libraries

V547 Expression is always true. Probably the '&&' operator should be used here. incidenceformatter.cpp 2684

enum PartStat { .... Accepted, Tentative, .... }; static QString formatICalInvitationHelper(....) { .... a = findDelegatedFromMyAttendee( inc ); if ( a ) { if ( a->status() != Attendee::Accepted || //<== a->status() != Attendee::Tentative ) { //<== html += responseButtons( inc, rsvpReq, rsvpRec, helper ); break; } } .... } 

The expression is always true. The reason may be a typo or incorrect programmer logic. It may be necessary to use the '&&' operator here.

Similar suspicious place:

• V547 Expression is always true. Probably the '&&' operator should be used here. incidenceformatter.cpp 3293

V593 Consider reviewing the A = B == C 'kind. The expression is calculated as the following: 'A = (B == C)'. kio_ldap.cpp 535

 void LDAPProtocol::del( const KUrl &_url, bool ) { .... if ( (id = mOp.del( usrc.dn() ) == -1) ) { LDAPErr(); return; } ret = mOp.waitForResult( id, -1 ); .... } 

The priority of the comparison operator (==) is higher than the priority of the assignment operator (=). Due to luck, the condition is fulfilled as conceived, but then the incorrect value of the identifier 'id', equal to 0, is used.

V595 The 'incBase' pointer was used before it was verified against nullptr. Check lines: 2487, 2491. incidenceformatter.cpp 2487

 static QString formatICalInvitationHelper(....) { .... incBase->shiftTimes( mCalendar->timeSpec(), ....); Incidence *existingIncidence = 0; if ( incBase && helper->calendar() ) { .... } .... } 

The dereferencing of the 'incBase' pointer is done before checking it.
')
V622 Consider inspecting the 'switch' statement. It's possible that the operator is missing. listjob.cpp 131

 void ListJob::doStart() { Q_D( ListJob ); switch ( d->option ) { break; //<== case IncludeUnsubscribed: d->command = "LIST"; break; case IncludeFolderRoleFlags: d->command = "XLIST"; break; case NoOption: default: d->command = "LSUB"; } .... } 

In the 'switch' statement block, the first statement is not a 'case'. This leads to the fact that the code fragment will never get control. In the best case, the operator 'break' remained after the incomplete removal of the old condition, but in the worst case, another 'case' was missing here.

V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'lexBuf.strs' is lost. Consider assigning realloc () to a temporary pointer. vcc.y 638

 static void lexAppendc(int c) { lexBuf.strs = (char *) realloc(lexBuf.strs, (size_t) .... + 1); lexBuf.strs[lexBuf.strsLen] = c; .... } 

This expression is potentially dangerous: it is recommended that the result of the realloc function be stored in another variable. The realloc () function changes the size of a block of memory. In case of an error, the pointer to the old memory area will be lost.

Anyway, this code is bad. There is no proof that the function realloc () returned. The pointer is immediately dereferenced in the next line.

Similar places:

• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'mods' is lost. Consider assigning realloc () to a temporary pointer. ldapoperation.cpp 534
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'mods [i] -> mod_vals.modv_bvals' is lost. Consider assigning realloc () to a temporary pointer. ldapoperation.cpp 579
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'ctrls' is lost. Consider assigning realloc () to a temporary pointer. ldapoperation.cpp 624
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'fp-> s' is lost. Consider assigning realloc () to a temporary pointer. vobject.c 1055
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'lexBuf.strs' is lost. Consider assigning realloc () to a temporary pointer. vcc.y 635
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'lexBuf.strs' is lost. Consider assigning realloc () to a temporary pointer. vcc.y 643
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'bytes' is lost. Consider assigning realloc () to a temporary pointer. vcc.y 928
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'fp-> s' is lost. Consider assigning realloc () to a temporary pointer. vobject.c 1050

KDE Base Libraries

V523 The 'then' statement is equivalent to the 'else' statement. kconfig_compiler.cpp 1051

 QString newItem( const QString &type, ....) { QString t = "new "+cfg.inherits+"::Item" + ....; if ( type == "Enum" ) t += ", values" + name; if ( !defaultValue.isEmpty() ) { t += ", "; if ( type == "String" ) t += defaultValue; //<== else t+= defaultValue; //<== } t += " );"; return t; } 

The same true and false branch of the 'if' operator looks extremely suspicious. If the code still does not contain a typo, then it can be simplified, for example, as follows:

 if ( !defaultValue.isEmpty() ) t += ", " + defaultValue; 

Similar place:

• V523 The 'then' statement is equivalent to the 'else' statement. installation.cpp 589

V595 The 'priv-> slider' pointer was used before it was verified against nullptr. Check lines: 786, 792. knuminput.cpp 786

 void KDoubleNumInput::spinBoxChanged(double val) { .... const double slidemin = priv->slider->minimum(); //<== const double slidemax = priv->slider->maximum(); //<== .... if (priv->slider) { //<== priv->slider->blockSignals(true); priv->slider->setValue(qRound(slidemin + rel * (....))); priv->slider->blockSignals(false); } } 

The pointer de priv ’is dereferenced before checking it

Similar dangerous places:

• V595 The 'm_instance' pointer was used before it was counted against nullptr. Check lines: 364, 376. ksystemtimezone.cpp 364
• V595 Check lines: 778, 783. knewfilemenu.cpp 778

V646 Consider inspecting the application's logic. It's possible that 'else' keyword is missing. karchive.cpp 187

 *bool KArchive::close() { .... // if d->saveFile is not null then it is equal to d->dev. if ( d->saveFile ) { closeSucceeded = d->saveFile->finalize(); delete d->saveFile; d->saveFile = 0; } if ( d->deviceOwned ) { //<== delete d->dev; // we created it ourselves in open() } .... } 

This code snippet may indicate the omission of the 'else' keyword, or it is simply extremely delusive and confusing code design.

V655 The strings was concatenated but are not utilized. Consider inspecting the expression. entrydetailsdialog.cpp 225

 void EntryDetails::updateButtons() { .... foreach (....) { QString text = info.name; if (!info.distributionType.trimmed().isEmpty()) { text + " (" + info.distributionType.trimmed() + ")";//<== } QAction* installAction = installMenu->addAction(KIcon("dialog-ok"), text); installAction->setData(info.id); } .... } 

The analyzer detected an unused union of string variables. Perhaps the code should be like this:

 text += " (" + info.distributionType.trimmed() + ")"; 

Similar places:

• V655 The strings was concatenated but are not utilized. Consider inspecting the expression. itemsgridviewdelegate.cpp 365
• V655 The strings was concatenated but are not utilized. Consider inspecting the expression. itemsviewdelegate.cpp 159

V705 It is possible that “else” block was forgotten or commented out, thus altering the program's operation logics. entrydetailsdialog.cpp 149

 void EntryDetails::entryChanged(const KNS3::EntryInternal& entry) { .... if(m_entry.previewUrl(EntryInternal::PreviewSmall1).isEmpty()){ ui->previewBig->setVisible(false); } else //<== if (!m_entry.previewUrl((....)type).isEmpty()) { .... } .... } 

The design of this code fragment also looks ambiguous. Was there an “else if” construct planned here, or just forgot to remove the 'else'?

V612 An unconditional 'return' within a loop. bufferfragment_p.h 94

 BufferFragment split(char c, unsigned int* start) { while (*start < len) { int end = indexOf(c, *start); if (end == -1) end = len; BufferFragment line(d + (*start), end - (*start)); *start = end + 1; return line; } return BufferFragment(); } 

Was it worth writing a loop because of one iteration? Or do not have a conditional operator?

V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'd' is lost. Consider assigning realloc () to a temporary pointer. netwm.cpp 596

 template <class Z> void NETRArray<Z>::reset() { sz = 0; capacity = 2; d = (Z*) realloc(d, sizeof(Z)*capacity); memset( (void*) d, 0, sizeof(Z)*capacity ); } 

As in KDE PIM Libraries, it is not recommended to use a single pointer with the realloc () function, as the pointer to the old memory area may be lost if the memory cannot be increased.

Similar places:

• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'handlers' is lost. Consider assigning realloc () to a temporary pointer. kxerrorhandler.cpp 94
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'buffer' is lost. Consider assigning realloc () to a temporary pointer. netwm.cpp 528
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'd' is lost. Consider assigning realloc () to a temporary pointer. netwm.cpp 608
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'ptr' is lost. Consider assigning realloc () to a temporary pointer. kdesu_stub.c 119
• V701 realloc () possible leak: when realloc () fails in allocating memory, the original pointer 'addr.generic' is lost. Consider assigning realloc () to a temporary pointer. k3socketaddress.cpp 372

KDE Base Apps

V501 There are identical sub-expressions' mimeData-> hasFormat (QLatin1String ("application / x-kde-ark-dndextract-service"))). iconview.cpp 2357

 void IconView::dropEvent(QGraphicsSceneDragDropEvent *event) { .... if (mimeData->hasFormat(QLatin1String( "application/x-kde-ark-dndextract-service")) && //<== mimeData->hasFormat(QLatin1String( "application/x-kde-ark-dndextract-service"))) //<== { const QString remoteDBusClient = mimeData->data( QLatin1String("application/x-kde-ark-dndextract-service")); const QString remoteDBusPath = mimeData->data( QLatin1String("application/x-kde-ark-dndextract-path")); .... } .... } 

The analyzer detected the same conditional expression. According to the body of the conditional operator, it is logical to assume that the condition should be like this:

 if (mimeData->hasFormat(QLatin1String( "application/x-kde-ark-dndextract-service")) && //<== mimeData->hasFormat(QLatin1String( "application/x-kde-ark-dndextract-path "))) //<== { .... } 

V595 The 'm_view' pointer was used before it was verified against nullptr. Check lines: 797, 801. kitemlistcontroller.cpp 797

 bool KItemListController::mouseDoubleClickEvent(....) { const QPointF pos = transform.map(event->pos()); const int index = m_view->itemAt(pos); // Expand item if desired - See Bug 295573 if (m_mouseDoubleClickAction != ActivateItemOnly) { if (m_view && m_model && ....) { const bool expanded = m_model->isExpanded(index); m_model->setExpanded(index, !expanded); } } .... } 

In KDE projects, there are many places where the pointer that came into the function is first used to initialize local variables, and later it is checked before dereferencing.

V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 410, 412. kebsearchline.cpp 410

 void KViewSearchLine::slotColumnsRemoved(const QModelIndex &, int first, int last) { if(d->treeView) updateSearch(); else { if(d->listView->modelColumn() >= first && d->listView->modelColumn() <= last) { if(d->listView->modelColumn()>last) //<== kFatal()<<"...."<<endl; updateSearch(); } } } 

Nested condition will always be false. It can be assumed that such a condition was relevant before some edits.

V654 The condition 'state! = 1' of loop is always true. passwd.cpp 255

 int PasswdProcess::ConversePasswd(....) { .... state = 0; while (state != 1) { line = readLine(); if (line.isNull()) { // No more input... OK return 0; } if (isPrompt(line, "password")) { // Uh oh, another prompt. Not good! kill(m_Pid, SIGKILL); waitForChild(); return PasswordNotGood; } m_Error += line + '\n'; // Collect error message } .... } 

The value of the 'state' variable does not change in the loop, therefore, the only condition to stop is the call 'return'.

KDE Development

V501 There are identical sub-expressions 'file == rhs.file' operator. pp-macro.cpp 44

 bool pp_macro::operator==(const pp_macro& rhs) const { if(completeHash() != rhs.completeHash()) return false; return name == rhs.name && file == rhs.file && //<== file == rhs.file && //<== sourceLine == rhs.sourceLine && defined == rhs.defined && hidden == rhs.hidden && function_like == rhs.function_like && variadics == rhs.variadics && fixed == rhs.fixed && defineOnOverride == rhs.defineOnOverride && listsEqual(rhs); } 

The analyzer found several places with repeated conditional expressions. Some of this may well be a serious typo.

More places like this:

• V501 There are identical sub-expressions "tokenKind == Token_not_eq" operator. builtinoperators.cpp 174
• V501 There are identical sub-expressions '! Context-> owner ()' operator. typeutils.cpp 194

V595 The 'parentJob () -> cpp ()' pointer was used against nullptr. Check lines: 437, 438. cppparsejob.cpp 437

 void CPPInternalParseJob::run() { .... QReadLocker lock(parentJob()->parentPreprocessor() ? 0: parentJob()->cpp()->language()->parseLock()); //<== if(.... || !parentJob()->cpp()) //<== return; .... } 

And in this project there was a place with a pointer dereference until it was checked.

Another place:

• V595 The 'parentContext ()' pointer was used before it was verified against nullptr. Check lines: 692, 695. context.cpp 692

V564 The '&' operator is applied to bool type value. You've probably forgotten to include the operator. usedecoratorvisitor.cpp 40

 DataAccess::DataAccessFlags typeToDataAccessFlags(....) { DataAccess::DataAccessFlags ret = DataAccess::Read; TypePtr< ReferenceType > reftype=type.cast<ReferenceType>(); if(reftype && reftype->baseType() && !reftype->baseType()->modifiers() & //<== AbstractType::ConstModifier) ret |= DataAccess::Write; return ret; } 

Authors should know whether the error is here or not, but the '&' operator looks suspicious. Note that the expression "! Reftype-> baseType () -> modifiers ()" is of the type 'bool'.

V555 The expression 'm_pos - backOffset> 0' will work as 'm_pos! = BackOffset'. pp-stream.cpp 225

 unsigned int rpp::Stream::peekLastOutput(uint backOffset) const { if(m_pos - backOffset > 0) return m_string->at(m_pos - backOffset - 1); return 0; } 

It is not entirely correct to compare the difference between unsigned numbers and zero, because a negative result can be interpreted as a very large positive number. In order not to get a giant index in the condition body, it is better to write the condition as follows:

 if(m_pos > backOffset) return m_string->at(m_pos - backOffset - 1); 

Similar place:

• V555 The expression 'nextOffset - currentOffset> 0' will work as 'nextOffset! = CurrentOffset'. pp-location.cpp 211

Conclusion

The huge audience of KDE users and product developers plays a big role in terms of testing, but it is also worth paying attention to various code analyzers. However, judging by the publications on the Internet, at least Coverity is already used to check the KDE source codes. It is because of this that PVS-Studio finds very few suspicious places.

Using static analysis regularly, you can save a lot of time on solving more useful tasks. Also, control over the quality of the code can be shifted to others, for example, using the new service: regular auditing of C / C ++ code .

This article is in English.

If you want to share this article with an English-speaking audience, then please use the link to the translation: Svyatoslav Razmyslov. The Unicorn Getting Interested in KDE .