Проверка исходного кода свободного графического редактора Krita 4.0
Не так давно состоялся релиз новой версии свободного графического редактора Krita 4.0. Самое время проверить этот проект с помощью PVS-Studio.
Введение
Примечательно, что разработчики уже использовали PVS-Studio в далеком 2015 году на версии Krita 2.9.2 и успешно исправляли с его помощью ошибки. Однако, видимо, с тех пор анализатор не использовали. Во всех наших статьях мы говорим о том, что важны именно регулярные проверки, ведь если бы разработчики продолжали пользоваться PVS-Studio, то ошибки, о которых я расскажу в этой статье, попросту не попали бы в релиз.
Бесполезный range-based for
Предупреждение PVS-Studio: V714 Variable row is not passed into foreach loop by a reference, but its value is changed inside of the loop. kis_algebra_2d.cpp 532
DecomposedMatix::DecomposedMatix(const QTransform &t0)
{
....
if (!qFuzzyCompare(t.m33(), 1.0)) {
const qreal invM33 = 1.0 / t.m33();
for (auto row : rows) { // <=
row *= invM33;
}
}
....
}
В данном примере программист, очевидно, хотел умножить каждый элемент контейнера rows на invM33, однако, этого не произойдёт. На каждой итерации цикла создаётся новая переменная с именем row, которая затем просто уничтожается. Чтобы исправить ошибку необходимо создавать ссылку на элемент, хранящийся в контейнере:
for (auto &row : rows) {
row *= invM33;
}Некорректные условия
Предупреждение PVS-Studio: V547 Expression 'j == 0' is always false. KoColorSpace.cpp 218
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] = ....
}
}
}
....
}
Программа никогда не зайдет в блок под условием j==0, поскольку это условие всегда ложно из-за того, что выше в цикле for накладывается ограничение j > 0.
Аналогичные предупреждения анализатора:
- 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: V560 A part of conditional expression is always true. KoTextLayoutArea.cpp 1622
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()) { // <=
....
}
}
Если присмотреться, то можно заметить, что внутри данного сложного условия находится проверка вида (! a || a):
d->documentLayout->changeTracker()->displayChanges() ||
!d->documentLayout->changeTracker()->displayChanges()
Такое условие является всегда истинным, из-за чего вся эта большая проверка становится бессмысленной, о чем и сообщает анализатор.
Предупреждение PVS-Studio: V547 Expression 'n == 128' is always false. compression.cpp 110
Предупреждение PVS-Studio: V547 Expression 'n > 128' is always false. compression.cpp 112
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;
....
}
....
}
В данном примере в переменную n типа qint32 записывается значение типа const char, получаемое при разыменовании указателя src, поэтому диапазон допустимых значений переменной n: [-128; 127]. Затем переменная n сравнивается с числом 128, хотя понятно, что результат такой проверки всегда false.
Примечание: Проект собирается без -funsigned-char.
Предупреждение PVS-Studio: V590 Consider inspecting the 'state == (- 3) || state!= 0' expression. The expression is excessive or contains a misprint. psd_pixel_utils.cpp 335
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);
}
Здесь перемудрили со вторым условием, из-за чего оно стало избыточным. Предположу, что корректный вариант выглядит следующим образом:
do {
state = inflate(&stream, Z_PARTIAL_FLUSH);
if(state == Z_STREAM_END)
break;
if(state != Z_OK)
break;
} while (stream.avail_out > 0);
Предупреждение PVS-Studio: V547 Expression is always false. SvgTextEditor.cpp 472
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);
}
}
Анализатор обнаружил условие вида (a > b && a < b), которое, очевидно, является всегда ложным. Затрудняюсь сказать, что именно хотел написать автор, однако, данный код однозначно является ошибочным и нуждается в исправлении.
Очепятки
Предупреждение PVS-Studio: V547 Expression is always true. Probably the '&&' operator should be used here. KoResourceItemChooser.cpp 408
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!= const_1 || a!= const_2, которое является всегда истинным.
Предупреждение PVS-Studio: V547 Expression is always true. Probably the '&&' operator should be used here. KoSvgTextShapeMarkupConverter.cpp 1000
QString KoSvgTextShapeMarkupConverter::style(....)
{
....
if (format.underlineStyle() != QTextCharFormat::NoUnderline ||
format.underlineStyle() != QTextCharFormat::SpellCheckUnderline)
{
....
}
....
}
Случай аналогичный предыдущему: перепутали логический оператор и вместо && написали ||.
Предупреждение PVS-Studio: 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
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("...."));
}
}
Анализатор обнаружил ситуацию, когда слева и справа от оператора || находятся одинаковые выражения. Если взглянуть на перечисление DynamicSensorType:
enum DynamicSensorType {
FUZZY_PER_DAB,
FUZZY_PER_STROKE,
SPEED,
FADE,
....
UNKNOWN = 255
};
то становится ясно, что справа, скорее всего, хотели написать FUZZY_PER_STROKE, а не FUZZY_PER_DAB.
Статические анализаторы хорошо справляются с поиском подобных ошибок, а вот на код-ревью их легко проглядеть, особенно, когда нужно просмотреть большое количество изменений.
Ошибки из-за Copy-Paste
Предупреждение PVS-Studio: V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value: d→paragraphStylesDotXmlStyles.values (). KoTextSharedLoadingData.cpp 594
QList
KoTextSharedLoadingData::paragraphStyles(bool stylesDotXml) const
{
return stylesDotXml ?
d->paragraphStylesDotXmlStyles.values() :
d->paragraphStylesDotXmlStyles.values(); // <=
}
Здесь, скорее всего, скопировали блок then в тернарном операторе и забыли изменить имя вызываемого метода, из-за чего, в независимости от условия, всегда будет возвращаться одно значение.
Судя по предыдущему методу:
KoParagraphStyle *
KoTextSharedLoadingData::paragraphStyle(const QString &name,
bool stylesDotXml) const
{
return stylesDotXml ?
d->paragraphStylesDotXmlStyles.value(name) :
d->paragraphContentDotXmlStyles.value(name);
}
в блоке else нужно написать paragraphContentDotXmlStyles, вместо paragraphStylesDotXmlStyles.
Предупреждение PVS-Studio: V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value: qFloor (axis). kis_transform_worker.cc 456
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);
....
}
Еще одно срабатывание, очень похожее на предыдущее. Возможно, в блоке then первого тернарного оператора хотели написать qCeil (axis), а не qFloor (axis), либо же условие здесь вообще лишнее.
Предупреждение PVS-Studio: 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
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(); // <=
....
}
Данный код выглядит весьма подозрительно, так как, скорее всего, при написании формулы для vy скопировали предыдущую строку, но забыли поменять вызовы x () на y (). В случае, если ошибки здесь все-таки нет, лучше переписать код так:
qreal vx = 3 * curve[2].x() - 2 * curve[3].x() - curve[0].x();
qreal vy = vx;
Предупреждение PVS-Studio: V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 675, 679. KoTableCellStyle.cpp 679
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");
}
....
}
Два раза выполняется одна и та же проверка. В этом методе либо скопировали лишнего, либо что-то перепутали. Если ошибки здесь нет, то стоит удалить дублирующийся код.
Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. kis_processing_applicator.cpp 227
void KisProcessingApplicator::applyVisitorAllFrames(....)
{
KisLayerUtils::FrameJobs jobs;
if (m_flags.testFlag(RECURSIVE)) {
KisLayerUtils::updateFrameJobsRecursive(&jobs, m_node); // <=
} else {
KisLayerUtils::updateFrameJobsRecursive(&jobs, m_node); // <=
}
....
}
Скорее всего, здесь скопировали код из блока then в блок else и забыли поменять вызываемый метод. Судя по коду проекта, в ветке else наверняка хотели написать KisLayerUtils: updateFrameJobs.
Дублирующееся условие (ошибка в условии)
Предупреждение PVS-Studio: 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
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);
....
}
Здесь два раза выполняется одна и та же проверка. Скорее всего, во втором случае нужно было написать что-то вроде «sender-title».
Ошибки при работе с константами из перечисления
Предупреждение PVS-Studio: V768 The enumeration constant 'BatchMode' is used as a variable of a Boolean-type. KisMainWindow.cpp 811
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 — это элемент перечисления OpenFlag со значением 0×2:
enum OpenFlag {
None = 0,
Import = 0x1,
BatchMode = 0x2,
RecoveryFile = 0x4
};
Однако в данном примере с ним работают, как будто это переменная. В результате получается, что часть условия всегда истинна.
При этом в коде выше с BatchMode работают корректно:
if (flags & BatchMode) {
newdoc->setFileBatchMode(true);
}
Из этого можно сделать вывод, что хотели написать нечто подобное:
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: V768 The enumeration constant 'State_Active' is used as a variable of a Boolean-type. KisOpenPane.cpp 104
void paint(....) const override
{
QStyledItemDelegate::paint(painter, option, index);
if(!(option.state & (int)(QStyle::State_Active && // <=
QStyle::State_Enabled))) // <=
{
....
}
}
В данном случае, судя по всему, перепутали операторы и вместо | внутри маски использовали оператор &&. Думаю, что исправленный вариант должен выглядеть следующим образом:
void paint(....) const override
{
QStyledItemDelegate::paint(painter, option, index);
if(!(option.state & (int)(QStyle::State_Active |
QStyle::State_Enabled)))
{
....
}
}Подозрительные повторные присваивания
Предупреждение PVS-Studio: V519 The 'value' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 61, 66. kis_draggable_tool_button.cpp 66
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;
}
Переменной value присваивают некоторое значение внутри условия, однако, затем сразу же перезаписывают это значение. Скорее всего, здесь есть какая-то ошибка.
Предупреждение PVS-Studio: V519 The 'uf.f' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 263, 265. lut.h 265
Предупреждение PVS-Studio: V519 The 'uf.f' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 271, 273. lut.h 273
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;
}
....
}
Переменной uf.f два раза подряд присваиваются разные значения. Это подозрительно, и вполне возможно, что хотели присвоить значение какой-то другой переменной.
Возможно, пропущен else
Предупреждение PVS-Studio: V646 Consider inspecting the application’s logic. It’s possible that 'else' keyword is missing. SvgStyleWriter.cpp 82
void SvgStyleWriter::saveSvgBasicStyle(KoShape *shape,
SvgSavingContext &context)
{
if (!shape->isVisible(false)) {
....
} if (shape->transparency() > 0.0) { // <=
....
}
}
Здесь, возможно, забыли ключевое слово else. Даже если ошибки здесь нет, стоит поправить форматирование кода, чтобы не смущать анализатор и других программистов.
Аналогичное предупреждение:
- V646 Consider inspecting the application’s logic. It’s possible that 'else' keyword is missing. transform_stroke_strategy.cpp 166
Проблемы с нулевыми указателями
Предупреждение PVS-Studio: V595 The 'l' pointer was utilized before it was verified against nullptr. Check lines: 428, 429. kis_node_manager.cpp 428
void KisNodeManager::moveNodeAt(....)
{
....
KisLayer *l = qobject_cast(parent.data());
KisSelectionMaskSP selMask = l->selectionMask(); // <=
if (m && m->active() && l && l->selectionMask()) // <=
selMask->setActive(false);
....
}
Здесь указатель l сначала разыменовывают и только потом проверяют его на nullptr.
Аналогичные предупреждения анализатора:
- 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: V1004 The 'sb' pointer was used unsafely after it was verified against nullptr. Check lines: 665, 670. KisView.cpp 670
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());
}
}
Анализатор считает небезопасным подобное использование указателя sb после проверки его на nullptr. И действительно, в случае, если указатель будет нулевым (а это допускается, раз такое условие написано выше), то при вызове sb→isHidden () может произойти разыменование нулевого указателя. В качестве исправления можно добавить проверку на nullptr и во второе условие, либо еще как-то обработать эту ситуацию.
Аналогичное предупреждение анализатора:
- V1004 The 'd→viewManager' pointer was used unsafely after it was verified against nullptr. Check lines: 338, 365. KisView.cpp 365
Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'slot' might take place. kis_spriter_export.cpp 568
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 = .... // <=
}
....
}
В данном примере гарантированно произойдет разыменование нулевого указателя slot, что в свою очередь приводит к неопределенному поведению программы.
Утечки памяти
Предупреждение PVS-Studio: V773 The function was exited without releasing the 'svgSymbol' pointer. A memory leak is possible. SvgParser.cpp 681
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; // <=
....
}
В этом примере при выходе из метода забыли освободить память, выделенную под svgSymbol. Это утечка памяти. В проекте много подобных утечек, но они достаточно однотипны, так что останавливаться на них я не буду.
Аналогичные предупреждения анализатора:
- 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
- …
Проверки на nullptr после new
Предупреждение PVS-Studio: 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
bool KoPathShape::separate(QList & separatedPaths)
{
....
Q_FOREACH (KoSubpath* subpath, d->subpaths) {
KoPathShape *shape = new KoPathShape();
if (! shape) continue; // <=
....
}
}
Эта рубрика в наших статьях, кажется, уже стала постоянной. Бессмысленно проверять указатель на nullptr, если память была выделена с помощью оператора new. При невозможности выделить память, оператор new генерирует исключение std: bad_alloc (), а не возвращает nullptr. Чтобы исправить этот код, можно добавить обработчик исключения, либо использовать new (std: nothrow).
Аналогичные предупреждения анализатора:
- 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
- …
Рефакторинг
Предупреждение PVS-Studio: V728 An excessive check can be simplified. The '(A && ! B) || (! A && B)' expression is equivalent to the 'bool (A) != bool (B)' expression. kis_filter_weights_buffer.h 216
#define SANITY_ZEROS()
.... \
if ((m_filterWeights[i].weight[j] && \
!m_filterWeights[i].weight[idx2]) || \
(!m_filterWeights[i].weight[j] && \
m_filterWeights[i].weight[idx2])) { \
.... \
} \
.... \
Выражение вида a && ! b || ! a && b является избыточным и может быть упрощено до a!= b.
Исправленный вариант:
#define SANITY_ZEROS()
.... \
if ((m_filterWeights[i].weight[j] != \
!m_filterWeights[i].weight[idx2])) { \
.... \
} \
.... \
Аналогичные предупреждения анализатора:
- V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '! nodeJuggler' and 'nodeJuggler'. kis_node_manager.cpp 809
- 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: V501 There are identical sub-expressions to the left and to the right of the '&&' operator: ! iterator.atEnd () &&! iterator.atEnd () KoTextDebug.cpp 867
void KoTextDebug::dumpFrame(const QTextFrame *frame, QTextStream &out)
{
....
QTextFrame::iterator iterator = frame->begin();
for (; !iterator.atEnd() && !iterator.atEnd(); ++iterator) { // <=
....
}
....
}
Стоит проверить условие цикла for на наличие ошибок. Если ошибок нет, стоит убрать дублирующую проверку.
Аналогичное предупреждение анализатора:
- 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: 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
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; // <=
....
}
Здесь выделили и освободили память под объект cmd, но так ни разу его и не использовали.
Предупреждение PVS-Studio: V732 Unary minus operator does not modify a bool type value. Consider using the '!' operator. kis_equalizer_slider.cpp 75
QRect KisEqualizerSlider::Private::boundingRect() const
{
QRect bounds = q->rect().adjusted(0, 0, -isRightmost, -1);
return bounds;
}
В данном примере переменная isRightmost имеет тип bool. Используя унарный минус, переменную неявно преобразовывают к типу int и передают получившееся число в метод adjusted (). Такой код усложняет понимание. Явное лучше, чем неявное, так что, думаю, стоит переписать этот фрагмент так:
QRect KisEqualizerSlider::Private::boundingRect() const
{
QRect bounds = q->rect().adjusted(0, 0, isRightmost ? -1 : 0, -1);
return bounds;
}
Аналогичные предупреждения анализатора:
- 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
Заключение
В заключение мне хочется обратиться к разработчикам Krita и предложить им бесплатно возобновить использование нашего анализатора.
С тех пор, как разработчики последний раз использовали PVS-Studio, у нас появились версии для Linux и MacOS (сам я проверял этот проект в Linux), а также значительно улучшился анализ.
Кроме того, приглашаю всех желающих скачать и попробовать PVS-Studio на коде своего проекта.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Egor Bredikhin. Check of the Krita 4.0 Free Source Code Graphics Editor
