# Check of the Krita 4\.0 Open Source Graphics Editor Not so long ago, a new version of the Krita 4\.0 free graphics editor was released\. It's high time to check this project using PVS\-Studio\. ![0569_Krita/image1.png](https://import.viva64.com/docx/blog/0569_Krita/image1.png) ## Introduction It's quite remarkable, that developers have already used PVS\-Studio in far 2015 for the version Krita 2\.9\.2 and have successfully fixed the [bugs](https://github.com/KDE/krita/search?q=pvs-studio&type=Commits) with the help of it\. However, it seems like since that time they haven't used the analyzer\. In many of our articles, we often say that regular checks are really important, because if the developers had continued using PVS\-Studio, the errors, which I'm considering in this article, simply wouldn't have got in the release\. ## Useless Range\-based for **PVS\-Studio warning**: [V714](https://pvs-studio.com/en/docs/warnings/v714/) Variable row is not passed into for each loop by a reference, but its value is changed inside of the loop\. kis\_algebra\_2d\.cpp 532 ```cpp DecomposedMatix::DecomposedMatix(const QTransform &t0) { .... if (!qFuzzyCompare(t.m33(), 1.0)) { const qreal invM33 = 1.0 / t.m33(); for (auto row : rows) { // <= row *= invM33; } } .... } ``` In this example, a programmer obviously wanted to multiply each element of the container _rows_ by _invM33_, however, this will not happen\. On each iteration of the loop, a new variable with the name _row_ is created and then simply removed\. To correct this error, you must create a reference to an element stored in a container: ```cpp for (auto &row : rows) { row *= invM33; } ``` ## Incorrect Conditions **PVS\-Studio warning**: [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'j \=\= 0' is always false\. KoColorSpace\.cpp 218 ```cpp QPolygonF KoColorSpace::estimatedTRCXYY() const { .... for (int j = 5; j>0; j--) { channelValuesF.fill(0.0); channelValuesF[i] = ((max / 4)*(5 - j)); if (colorModelId().id() != "XYZA") { fromNormalisedChannelsValue(data, channelValuesF); convertPixelsTo(....); xyzColorSpace->normalisedChannelsValue(....); } if (j == 0) { // <= colorantY = channelValuesF[1]; if (d->colorants.size()<2) { d->colorants.resize(3 * colorChannelCount()); d->colorants[i] = .... d->colorants[i + 1] = .... d->colorants[i + 2] = .... } } } .... } ``` A program will never go in the block under the condition _j\=\=0_, because this condition is always false due to the fact that earlier in the _for _cycle there is a limitation _j \> 0_\. Similar analyzer warnings: * V547 Expression 'x < 0' is always false\. kis\_selection\_filters\.cpp 334 * V547 Expression 'y < 0' is always false\. kis\_selection\_filters\.cpp 342 **PVS\-Studio warning**: [V560](https://pvs-studio.com/en/docs/warnings/v560/) A part of conditional expression is always true\. KoTextLayoutArea\.cpp 1622 ```cpp qreal KoTextLayoutArea::addLine(QTextLine &line, FrameIterator *cursor, KoTextBlockData &blockData) { if (!d->documentLayout->changeTracker() || !d->documentLayout->changeTracker()->displayChanges() // <= || !d->documentLayout->changeTracker()->... || !d->documentLayout->changeTracker()->... || !d->documentLayout->changeTracker()->elementById(....) || !d->documentLayout->changeTracker()->elementById(....) || .... || d->documentLayout->changeTracker()->displayChanges()) { // <= .... } } ``` If you look closely, you will notice that within this complex condition there is a check of the type _\(\!a \|\| a\)_: ```cpp d->documentLayout->changeTracker()->displayChanges() || !d->documentLayout->changeTracker()->displayChanges() ``` This condition is always true, that is why this whole big check becomes meaningless, as reported by the analyzer\. **PVS\-Studio warning**: [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'n \=\= 128' is always false\. compression\.cpp 110 **PVS\-Studio warning**: [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'n \> 128' is always false\. compression\.cpp 112 ```cpp quint32 decode_packbits(const char *src, char* dst, quint16 packed_len, quint32 unpacked_len) { qint32 n; .... while (unpack_left > 0 && pack_left > 0) { n = *src; src++; pack_left--; if (n == 128) // <= continue; else if (n > 128) // <= n -= 256; .... } .... } ``` In this example, a value of the type _const char_, obtained by dereferencing the _src_ pointer, is written in the variable _n_ of the type _qint32_, that is why the range of the values for a variable _n _is as follows: \[\-128; 127\]\. Then the variable _n _is compared with the number 128, though it is clear that the result of such a check is always _false_\. Note: Project is built without _\-funsigned\-char_\. **PVS\-Studio warning**: [V590](https://pvs-studio.com/en/docs/warnings/v590/) Consider inspecting the 'state \=\= \(\- 3\) \|\| state \!\= 0' expression\. The expression is excessive or contains a misprint\. psd\_pixel\_utils\.cpp 335 ```cpp psd_status psd_unzip_without_prediction(psd_uchar *src_buf, psd_int src_len, psd_uchar *dst_buf, psd_int dst_len) { do { state = inflate(&stream, Z_PARTIAL_FLUSH); if(state == Z_STREAM_END) break; if(state == Z_DATA_ERROR || state != Z_OK) // <= break; } while (stream.avail_out > 0); } ``` Here the developers overthought with the second condition, that's why it became redundant\. I suppose that the correct version looks as follows: ```cpp do { state = inflate(&stream, Z_PARTIAL_FLUSH); if(state != Z_OK) break; } while (stream.avail_out > 0); ``` **PVS\-Studio warning**: [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression is always false\. SvgTextEditor\.cpp 472 ```cpp void SvgTextEditor::setTextWeightDemi() { if (m_textEditorWidget.richTextEdit->textCursor() .charFormat().fontWeight() > QFont::Normal && m_textEditorWidget.richTextEdit->textCursor() .charFormat().fontWeight() < QFont::Normal) { // <= setTextBold(QFont::Normal); } else { setTextBold(QFont::DemiBold); } } ``` The analyzer has detected the condition _\(a \> b && a < b\)_, which is, obviously, always false\. It's difficult to say what exactly the author wanted to write, but this code is clearly erroneous and needs correction\. ## Typos **PVS\-Studio warning**: [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression is always true\. Probably the '&&' operator should be used here\. KoResourceItemChooser\.cpp 408 ```cpp void KoResourceItemChooser::updatePreview(KoResource *resource) { .... if (image.format() != QImage::Format_RGB32 || // <= image.format() != QImage::Format_ARGB32 || // <= image.format() != QImage::Format_ARGB32_Premultiplied) { image = image.convertToFormat(....); } .... } ``` A programmer made a mistake and instead of writing _&&,_ wrote operator _\|\|_, leaving all its condition meaningless, because in the result he got the condition: _a_ _\!\=_ _const\_1_ _\|\|_ _a_ _\!\=_ _const\_2,_ which is always true\. **PVS\-Studio warning**: [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression is always true\. Probably the '&&' operator should be used here\. KoSvgTextShapeMarkupConverter\.cpp 1000 ```cpp QString KoSvgTextShapeMarkupConverter::style(....) { .... if (format.underlineStyle() != QTextCharFormat::NoUnderline || format.underlineStyle() != QTextCharFormat::SpellCheckUnderline) { .... } .... } ``` The case, similar to the previous one: instead of the _&&_ operator_ _wrote _\|\|_\. **PVS\-Studio warning**: [V501](https://pvs-studio.com/en/docs/warnings/v501/) There are identical sub\-expressions 'sensor\(FUZZY\_PER\_DAB, true\)' to the left and to the right of the '\|\|' operator\. kis\_pressure\_size\_option\.cpp 43 ```cpp void KisPressureSizeOption::lodLimitations(....) const { if (sensor(FUZZY_PER_DAB, true) || sensor(FUZZY_PER_DAB, true)) { l->limitations << KoID("size-fade", i18nc("....")); } if (sensor(FADE, true)) { l->blockers << KoID("....")); } } ``` The analyzer has detected a situation where on the left and on the right of the operator _\|\| _there are same expressions\. If you look at the _DynamicSensorType_ enumeration: ```cpp enum DynamicSensorType { FUZZY_PER_DAB, FUZZY_PER_STROKE, SPEED, FADE, .... UNKNOWN = 255 }; ``` it becomes clear that most likely a developer wanted to write _FUZZY\_PER\_STROKE _on the right_, _rather than _FUZZY\_PER\_DAB_\. Static analyzers are great at detecting such errors, whereas during code\-review it is easy to overlook them, especially when you have to view a big number of changes\. ## Errors Caused by Copy\-Paste ![0569_Krita/image2.png](https://import.viva64.com/docx/blog/0569_Krita/image2.png) **PVS\-Studio warning**: [V583](https://pvs-studio.com/en/docs/warnings/v583/) The '?:' operator, regardless of its conditional expression, always returns one and the same value: d\-\>paragraphStylesDotXmlStyles\.values\(\)\. KoTextSharedLoadingData\.cpp 594 ```cpp QList KoTextSharedLoadingData::paragraphStyles(bool stylesDotXml) const { return stylesDotXml ? d->paragraphStylesDotXmlStyles.values() : d->paragraphStylesDotXmlStyles.values(); // <= } ``` Most likely, a programmer copied the block _then_ in the ternary operator and forgot to change the name of the called method, because of which regardless of the condition, one value will always be returned\. Judging by the previous method: ```cpp KoParagraphStyle * KoTextSharedLoadingData::paragraphStyle(const QString &name, bool stylesDotXml) const { return stylesDotXml ? d->paragraphStylesDotXmlStyles.value(name) : d->paragraphContentDotXmlStyles.value(name); } ``` in the _else_ block one has to write _paragraphContentDotXmlStyles _instead of _paragraphStylesDotXmlStyles_\. **PVS\-Studio warning**: [V583](https://pvs-studio.com/en/docs/warnings/v583/) The '?:' operator, regardless of its conditional expression, always returns one and the same value: qFloor\(axis\)\. kis\_transform\_worker\.cc 456 ```cpp void mirror_impl(KisPaintDeviceSP dev, qreal axis, bool isHorizontal) { .... int leftCenterPoint = qFloor(axis) < axis ? qFloor(axis) : qFloor(axis); // <= int leftEnd = qMin(leftCenterPoint, rightEnd); int rightCenterPoint = qFloor(axis) < axis ? qCeil(axis) : qFloor(axis); int rightStart = qMax(rightCenterPoint, leftStart); .... } ``` Another triggering, very similar to the previous one\. Perhaps, in the then block of the first ternary operator a developer wanted to write _qCeil\(axis\), _not _qFloor\(axis\)_, or the condition here is even redundant\. **PVS\-Studio warning**: [V656](https://pvs-studio.com/en/docs/warnings/v656/) Variables 'vx', 'vy' are initialized through the call to the same function\. It's probably an error or un\-optimized code\. Check lines: 218, 219\. KarbonSimplifyPath\.cpp 219 ```cpp bool KarbonSimplifyPath::isSufficentlyFlat(QPointF curve[4]) { qreal ux = 3 * curve[1].x() - 2 * curve[0].x() - curve[3].x(); qreal uy = 3 * curve[1].y() - 2 * curve[0].y() - curve[3].y(); qreal vx = 3 * curve[2].x() - 2 * curve[3].x() - curve[0].x(); // <= qreal vy = 3 * curve[2].x() - 2 * curve[3].x() - curve[0].x(); // <= .... } ``` This code looks very suspicious, as, most likely, when writing formula for _vy, _one copied the previous line, but forgot to change the _x\(\) _calls to _y\(\)_\. In case if there is no error here, it's better to rewrite the code as follows: ```cpp qreal vx = 3 * curve[2].x() - 2 * curve[3].x() - curve[0].x(); qreal vy = vx; ``` **PVS\-Studio warning**: [V581](https://pvs-studio.com/en/docs/warnings/v581/) The conditional expressions of the 'if' statements situated alongside each other are identical\. Check lines: 675, 679\. KoTableCellStyle\.cpp 679 ```cpp void KoTableCellStyle::loadOdfProperties( KoShapeLoadingContext &context, KoStyleStack &styleStack) { .... if (styleStack.hasProperty(KoXmlNS::style, "print-content")) { setPrintContent(styleStack.property(KoXmlNS::style, "print-content") == "true"); } if (styleStack.hasProperty(KoXmlNS::style, "repeat-content")) // <= { setRepeatContent(styleStack.property(KoXmlNS::style, "repeat-content") == "true"); } if (styleStack.hasProperty(KoXmlNS::style, "repeat-content")) // <= { setRepeatContent(styleStack.property(KoXmlNS::style, "repeat-content") == "true"); } .... } ``` The same check is performed twice\. In this method, a developer either copied something extra, or mixed up something\. If there is no error, then one has to remove the repeated code\. **PVS\-Studio warning**: [V523](https://pvs-studio.com/en/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. kis\_processing\_applicator\.cpp 227 ```cpp void KisProcessingApplicator::applyVisitorAllFrames(....) { KisLayerUtils::FrameJobs jobs; if (m_flags.testFlag(RECURSIVE)) { KisLayerUtils::updateFrameJobsRecursive(&jobs, m_node); // <= } else { KisLayerUtils::updateFrameJobsRecursive(&jobs, m_node); // <= } .... } ``` Probably, one copied the code from the block _then_ to the block _else_ and forgot to change the called method\. Judging from the project code, a developer probably wanted to write _KisLayerUtils::updateFrameJobs _in the _else_ branch\. ## Repeated condition \(error in condition\) **PVS\-Studio warning**: [V517](https://pvs-studio.com/en/docs/warnings/v517/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 255, 269\. KoInlineTextObjectManager\.cpp 255 ```cpp void KoInlineTextObjectManager::documentInformationUpdated( const QString &info, const QString &data) { if (info == "title") // <= setProperty(KoInlineObject::Title, data); else if (info == "description") setProperty(KoInlineObject::Description, data); else if (info == "abstract") setProperty(KoInlineObject::Comments, data); else if (info == "subject") setProperty(KoInlineObject::Subject, data); else if (info == "keyword") setProperty(KoInlineObject::Keywords, data); else if (info == "creator") setProperty(KoInlineObject::AuthorName, data); else if (info == "initial") setProperty(KoInlineObject::AuthorInitials, data); else if (info == "title") // <= setProperty(KoInlineObject::SenderTitle, data); else if (info == "email") setProperty(KoInlineObject::SenderEmail, data); .... } ``` Here one check is performed twice\. Most likely, in the second case, one had to write something like _"sender\-title"_\. ## Errors occurring when working with enumeration constants **PVS\-Studio warning**: [V768](https://pvs-studio.com/en/docs/warnings/v768/) The enumeration constant 'BatchMode' is used as a variable of a Boolean\-type\. KisMainWindow\.cpp 811 ```cpp bool KisMainWindow::openDocument(const QUrl &url, OpenFlags flags) { if (!QFile(url.toLocalFile()).exists()) { if (!flags && BatchMode) { // <= QMessageBox::critical(0, i18nc("....", "Krita"), i18n("....", url.url())); } .... } .... } ``` _BatchMode _is the member of enumeration _OpenFlag_ with the value of _0x2_: ```cpp enum OpenFlag { None = 0, Import = 0x1, BatchMode = 0x2, RecoveryFile = 0x4 }; ``` In this example, however, it is handled as if it is a variable\. In the result, it turns out that a part of the condition is always true\. While in the above code, _BatchMode_ is handled correctly: ```cpp if (flags & BatchMode) { newdoc->setFileBatchMode(true); } ``` From this we can conclude that developers wanted to write something like this: ```cpp bool KisMainWindow::openDocument(const QUrl &url, OpenFlags flags) { if (!QFile(url.toLocalFile()).exists()) { if (!(flags & BatchMode)) { // <= QMessageBox::critical(0, i18nc("....", "Krita"), i18n("....", url.url())); } .... } .... } ``` **PVS\-Studio warning**: [V768](https://pvs-studio.com/en/docs/warnings/v768/) The enumeration constant 'State\_Active' is used as a variable of a Boolean\-type\. KisOpenPane\.cpp 104 ```cpp void paint(....) const override { QStyledItemDelegate::paint(painter, option, index); if(!(option.state & (int)(QStyle::State_Active && // <= QStyle::State_Enabled))) // <= { .... } } ``` In this case, apparently, the authors of the code mixed up the operators and instead of _\|_ inside the mask used the operator _&&_\. I think, the corrected version should be as follows: ```cpp void paint(....) const override { QStyledItemDelegate::paint(painter, option, index); if(!(option.state & (int)(QStyle::State_Active | QStyle::State_Enabled))) { .... } } ``` ## Suspicious Repeated Assignments **PVS\-Studio warning**: [V519](https://pvs-studio.com/en/docs/warnings/v519/) The 'value' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 61, 66\. kis\_draggable\_tool\_button\.cpp 66 ```cpp int KisDraggableToolButton::continueDrag(const QPoint &pos) { .... if (m_orientation == Qt::Horizontal) { value = diff.x(); // <= } else { value = -diff.y(); // <= } value = diff.x() - diff.y(); // <= return value; } ``` The variable _value_ is assigned a value within the condition, but then immediately the value is overwritten\. Most likely, there's some sort of an error\. **PVS\-Studio warning**: [V519](https://pvs-studio.com/en/docs/warnings/v519/) The 'uf\.f' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 263, 265\. lut\.h 265 **PVS\-Studio warning**: [V519](https://pvs-studio.com/en/docs/warnings/v519/) The 'uf\.f' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 271, 273\. lut\.h 273 ```cpp LutKey(float min, float max, float precision) : m_min(min), m_max(max), m_precision(precision) { .... if(m_min > 0 && m_max > 0) { uf.f = m_min; // <= m_tMin_p = uf.i >> m_shift; uf.f = m_max; // <= m_tMax_p = uf.i >> m_shift; m_tMin_n = m_tMax_p; m_tMax_n = m_tMax_p; } else if( m_max < 0) { uf.f = m_min; // <= m_tMax_n = uf.i >> m_shift; uf.f = m_max; // <= m_tMin_n = uf.i >> m_shift; m_tMin_p = m_tMax_n; m_tMax_p = m_tMax_n; } .... } ``` The variable _uf\.f_ is assigned with different values twice\. It is suspicious and it is quite possible that developers wanted to assign a value to another variable\. ## Perhaps Else Is Omitted Here **PVS\-Studio warning**: [V646](https://pvs-studio.com/en/docs/warnings/v646/) Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. SvgStyleWriter\.cpp 82 ```cpp void SvgStyleWriter::saveSvgBasicStyle(KoShape *shape, SvgSavingContext &context) { if (!shape->isVisible(false)) { .... } if (shape->transparency() > 0.0) { // <= .... } } ``` Here, perhaps, one forgot keyword _else_\. Even if there is no error, code formatting is worth fixing not to confuse the analyzer and other programmers\. A similar warning: * V646 Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. transform\_stroke\_strategy\.cpp 166 ## Problems with Null Pointers ![0569_Krita/image3.png](https://import.viva64.com/docx/blog/0569_Krita/image3.png) **PVS\-Studio warning**: [V595](https://pvs-studio.com/en/docs/warnings/v595/) The 'l' pointer was utilized before it was verified against nullptr\. Check lines: 428, 429\. kis\_node\_manager\.cpp 428 ```cpp void KisNodeManager::moveNodeAt(....) { .... KisLayer *l = qobject_cast(parent.data()); KisSelectionMaskSP selMask = l->selectionMask(); // <= if (m && m->active() && l && l->selectionMask()) // <= selMask->setActive(false); .... } ``` Here the pointer _l is_ first dereferenced and only after that checked for _nullptr_\. Similar analyzer warnings: * V595 The 'gradient' pointer was utilized before it was verified against nullptr\. Check lines: 164, 166\. kis\_gradient\_chooser\.cc 164 * V595 The 'm\_currentShape' pointer was utilized before it was verified against nullptr\. Check lines: 316, 325\. ArtisticTextTool\.cpp 316 * V595 The 'painter\(\)' pointer was utilized before it was verified against nullptr\. Check lines: 87, 89\. kis\_grid\_paintop\.cpp 87 * V595 The 'm\_optionsWidget' pointer was utilized before it was verified against nullptr\. Check lines: 193, 202\. kis\_tool\_move\.cc 193 * \.\.\.\. **PVS\-Studio warning**: [V1004](https://pvs-studio.com/en/docs/warnings/v1004/) The 'sb' pointer was used unsafely after it was verified against nullptr\. Check lines: 665, 670\. KisView\.cpp 670 ```cpp void KisView::slotSavingStatusMessage(const QString &text, int timeout, bool isAutoSaving) { QStatusBar *sb = statusBar(); if (sb) // <= sb->showMessage(text, timeout); KisConfig cfg; if (sb->isHidden() || // <= (!isAutoSaving && cfg.forceShowSaveMessages()) || (cfg.forceShowAutosaveMessages() && isAutoSaving)) { viewManager()->showFloatingMessage(text, QIcon()); } } ``` The analyzer considers unsafe such a usage of a pointer _sb _after its checking_ _for_ nullptr_\. Indeed, if the pointer is null \(which is allowed, once such a condition is written above\), then when calling _sb\-\>isHidden\(\), _null pointer dereference might occur\. As a hotfix, one can add a check for _nullptr_ in the second condition as well, or handle this situation differently\. The similar analyzer warning: * V1004 The 'd\-\>viewManager' pointer was used unsafely after it was verified against nullptr\. Check lines: 338, 365\. KisView\.cpp 365 **PVS\-Studio warning**: [V522](https://pvs-studio.com/en/docs/warnings/v522/) Dereferencing of the null pointer 'slot' might take place\. kis\_spriter\_export\.cpp 568 ```cpp KisImportExportFilter::ConversionStatus KisSpriterExport::convert( KisDocument *document, QIODevice *io, KisPropertiesConfigurationSP /*configuration*/) { .... SpriterSlot *slot = 0; // <= // layer.name format: "base_name bone(bone_name) slot(slot_name)" if (file.layerName.contains("slot(")) { int start = file.layerName.indexOf("slot(") + 5; int end = file.layerName.indexOf(')', start); slot->name = file.layerName.mid(start, end - start); // <= slot->defaultAttachmentFlag = .... // <= } .... } ``` In this example, a dereference of the null pointer _slot_ will certainly occur, which in turn results in undefined behavior\. ## Memory Leaks **PVS\-Studio warning**: [V773](https://pvs-studio.com/en/docs/warnings/v773/) The function was exited without releasing the 'svgSymbol' pointer\. A memory leak is possible\. SvgParser\.cpp 681 ```cpp bool SvgParser::parseSymbol(const KoXmlElement &e) { .... KoSvgSymbol *svgSymbol = new KoSvgSymbol(); // <= // ensure that the clip path is loaded in local coordinates system m_context.pushGraphicsContext(e, false); m_context.currentGC()->matrix = QTransform(); m_context.currentGC()->currentBoundingBox = QRectF(0.0, 0.0, 1.0, 1.0); QString title = e.firstChildElement("title").toElement().text(); KoShape *symbolShape = parseGroup(e); m_context.popGraphicsContext(); if (!symbolShape) return false; // <= .... } ``` In this example, when exiting the method one forgot to release the memory that has been allocated for _svgSymbol_\. This is a memory leak\. The project has many such leaks, but they are quite similar, so I won't discuss them a lot\. Similar analyzer warnings: * V773 The function was exited without releasing the 'ppmFlow' pointer\. A memory leak is possible\. kis\_ppm\_import\.cpp 249 * V773 The function was exited without releasing the 'keyShortcut' pointer\. A memory leak is possible\. kis\_input\_manager\_p\.cpp 443 * V773 The function was exited without releasing the 'layerRecord' pointer\. A memory leak is possible\. psd\_layer\_section\.cpp 109 * V773 The function was exited without releasing the 'filterStack' pointer\. A memory leak is possible\. FilterEffectResource\.cpp 139 * \.\.\.\. ## Checks for 'nullptr' After 'new' **PVS\-Studio warning**: [V668](https://pvs-studio.com/en/docs/warnings/v668/) There is no sense in testing the 'charStyle' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. CharacterGeneral\.cpp 153 ```cpp bool KoPathShape::separate(QList & separatedPaths) { .... Q_FOREACH (KoSubpath* subpath, d->subpaths) { KoPathShape *shape = new KoPathShape(); if (! shape) continue; // <= .... } } ``` It seems to me that this topic has become regular in our articles\. It is pointless to check a pointer for _nullptr _if the memory was allocated by the operator _new_\. If it impossible to allocate memory, the new operator throws an exception _std::bad\_alloc\(\), _it doesn't return _nullptr_\. To fix this code, you can add an exception handler, or use _new \(std: nothrow\)_\. Similar analyzer warnings: * V668 There is no sense in testing the 'factory' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. TestKoShapeFactory\.cpp 36 * V668 There is no sense in testing the 'parStyle' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. ParagraphGeneral\.cpp 199 * V668 There is no sense in testing the 'spline' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. multi\_bspline\_create\.cpp 460 * V668 There is no sense in testing the 'm\_currentStrategy' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. ConnectionTool\.cpp 333 * \.\.\.\. ## Refactoring **PVS\-Studio warning**: [V728](https://pvs-studio.com/en/docs/warnings/v728/) An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!nodeJuggler' and 'nodeJuggler'\. kis\_node\_manager\.cpp 809 ```cpp if (!nodeJuggler || // <= (nodeJuggler && // <= (nodeJuggler->isEnded() || !nodeJuggler->canMergeAction(actionName)))) { .... } ``` This check can be simplified: ```cpp if (!nodeJuggler || (nodeJuggler->isEnded() || !nodeJuggler->canMergeAction(actionName))) { .... } ``` Similar analyzer warning: * V728 An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!m\_currentFilterConfigWidget' and 'm\_currentFilterConfigWidget'\. kis\_filter\_option\.cpp 111 **PVS\-Studio warning**: [V501](https://pvs-studio.com/en/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '&&' operator: \!iterator\.atEnd\(\) &&\!iterator\.atEnd\(\) KoTextDebug\.cpp 867 ```cpp void KoTextDebug::dumpFrame(const QTextFrame *frame, QTextStream &out) { .... QTextFrame::iterator iterator = frame->begin(); for (; !iterator.atEnd() && !iterator.atEnd(); ++iterator) { // <= .... } .... } ``` It is worth checking the condition of the loop for errors\. If there are no errors, it is needed to remove a repeated check\. The similar analyzer warning: * V501 There are identical sub\-expressions to the left and to the right of the '&&' operator: \!iterator\.atEnd\(\) &&\!iterator\.atEnd\(\) KoTextDebug\.cpp 909 **PVS\-Studio warning**: [V799](https://pvs-studio.com/en/docs/warnings/v799/) The 'cmd' variable is not used after memory has been allocated for it\. Consider checking the use of this variable\. kis\_all\_filter\_test\.cpp 154 ```cpp bool testFilter(KisFilterSP f) { .... KisTransaction * cmd = new KisTransaction(kundo2_noi18n(f->name()), dev); // <= // Get the predefined configuration from a file KisFilterConfigurationSP kfc = f->defaultConfiguration(); QFile file(QString(FILES_DATA_DIR) + QDir::separator() + f->id() + ".cfg"); if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { //dbgKrita << "creating new file for " << f->id(); file.open(QIODevice::WriteOnly | QIODevice::Text); QTextStream out(&file); out.setCodec("UTF-8"); out << kfc->toXML(); } else { QString s; QTextStream in(&file); in.setCodec("UTF-8"); s = in.readAll(); //dbgKrita << "Read for " << f->id() << "\n" << s; kfc->fromXML(s); } dbgKrita << f->id();// << "\n" << kfc->toXML() << "\n"; f->process(dev, QRect(QPoint(0,0), qimage.size()), kfc); QPoint errpoint; delete cmd; // <= .... } ``` Here the memory was allocated and released for the object _cmd,_ but it wasn't used even once\. **PVS\-Studio warning**: [V732](https://pvs-studio.com/en/docs/warnings/v732/) Unary minus operator does not modify a bool type value\. Consider using the '\!' operator\. kis\_equalizer\_slider\.cpp 75 ```cpp QRect KisEqualizerSlider::Private::boundingRect() const { QRect bounds = q->rect().adjusted(0, 0, -isRightmost, -1); return bounds; } ``` In this example, the variable _isRightmost_ is of the type _bool_\. Using the unary minus, the variable is implicitly converted into the type _int_ and the resulting number is passed in the method _adjusted\(\)_\. Such code makes the understanding more difficult\. Explicit is better than implicit, so I think I'd rewrite this fragment like this: ```cpp QRect KisEqualizerSlider::Private::boundingRect() const { QRect bounds = q->rect().adjusted(0, 0, isRightmost ? -1 : 0, -1); return bounds; } ``` Similar analyzer warnings: * V732 Unary minus operator does not modify a bool type value\. Consider using the '\!' operator\. kis\_equalizer\_button\.cpp 66 * V732 Unary minus operator does not modify a bool type value\. Consider using the '\!' operator\. kis\_duplicateop\.cpp 222 ## Conclusion In conclusion, I would like to appeal to the developers of Krita and offer them to resume free [using](https://pvs-studio.com/en/blog/posts/0457/) of our analyzer\. Since the last time when the developers used PVS\-Studio, we have released the versions for Linux and macOS \(I tested this project in Linux myself\), and the analysis was significantly improved\. Besides, I suggest downloading and trying out [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) on the code of your project\.