Автор: Святослав Размыслов
Недавно мир узнал, что Digital Video, создатели TOONZ, и DWANGO, японский издатель, подписали соглашение о приобретении Dwango из Toonz, программного обеспечения для анимации, которое было независимо разработано Digital Video (Рим, Италия).
Digital Video и Dwango договорились закрыть сделку при условии, что Dwango опубликует и разработает платформу с открытым исходным кодом на основе Toonz (OpenToonz). Он будет включать функции, разработанные Studio Ghibli (* Toonz Ghibli Edition), который долгое время был пользователем Toonz. «Ходячий замок Хаула», «Унесенные призраками», «Понё на скале у моря» и многие другие фэнтези-фильмы - одни из самых известных фэнтезийных фильмов. Еще один мультфильм их производства «Футурама» вдохновил нашу команду на написание статьи об исходном коде OpenToonz.
Введение
OpenToonz - программа для создания 2D-анимации. Он основан на проекте Toonz, разработанном компанией Digital Video в Италии. Позже он был доработан Studio Ghibli, и уже много лет используется для создания своих работ. Помимо анимационных фильмов, этот проект также использовался для создания компьютерных игр, например, Discworld и Claw.
Стоит отметить, что цена комплекта была порядка 10000 долларов, но качество кода оставляет желать лучшего. Этот проект - настоящая находка для статического анализатора. Размер исходного кода OpenToonz составляет примерно 1/10 ядра FreeBSD, где с помощью PVS-Studio мы обнаружили более 40 серьезных ошибок, а здесь мы нашли гораздо больше!
OpenToonz был проверен в Visual Studio 2013 с помощью PVS-Studio версии 6.03, который поддерживает C / C ++ / C #, различные системы сборки и находится в стадии активной разработки. Этап компиляции уже вызвал много подозрений, когда я увидел количество предупреждений компилятора - к концу сборки их было 1211! Это показывает, что на код не обращали особого внимания! Более того, некоторые предупреждения компилятора были отключены #pragma warning, и даже там было несколько ошибок, о которых я расскажу позже. Эта статья будет немного нетипичной - мы представляем ошибки, обнаруженные в проекте, которые обычно характерны для начинающих программистов, которые только начали изучать C / C ++. Начну описание с предупреждений анализатора, связанных с некорректным использованием памяти и указателей.
Некорректная работа с памятью
V611 Память была выделена с помощью оператора new, но освобождена с помощью функции free. Рассмотрите возможность проверки логики работы за переменной row. motionblurfx.cpp 288
template <class T>
void doDirectionalBlur(....)
{
T *row, *buffer;
....
row = new T[lx + 2 * brad + 2]; // <=
if (!row)
return;
memset(row, 0, (lx + 2 * brad + 2) * sizeof(T));
....
free(row); // <=
r->unlock();
}
Анализатор обнаружил, что динамическая память выделяется и освобождается несовместимым образом. После вызова оператора new [] память необходимо освободить с помощью оператора delete []. Обратите внимание, что здесь используются квадратные скобки. Я хочу обратить на это ваше внимание неспроста - взгляните на следующий пример:
V611 Память была выделена с помощью оператора new T [], но была освобождена с помощью оператора delete. Рассмотрите возможность проверки этого кода. Возможно, лучше использовать delete [] uPrime;. tstroke.cpp 3353
double *reparameterize3D(....)
{
double *uPrime = new double[size]; // <=
for (int i = 0; i < size; i++) {
uPrime[i] = NewtonRaphsonRootFind3D(....);
if (!_finite(uPrime[i])) {
delete uPrime; // <=
return 0;
}
}
....
}
В C ++ операторы new / delete и new [] / delete [] используются парами. Использование разных операторов для выделения и освобождения динамической памяти является ошибкой. В приведенном выше коде память, выделенная для массива uPrime, не будет правильно освобождена.
К сожалению, этот фрагмент не единственный. Я записал еще 20 фрагментов в файл OpenToonz_V611.txt.
V554 Некорректное использование auto_ptr. Память, выделенная с помощью new [], будет очищена с помощью delete. screensavermaker.cpp 29
void makeScreenSaver(....)
{
....
std::auto_ptr<char> swf(new char[swfSize]);
....
}
Здесь у нас есть альтернативный вариант только что обнаруженной ошибки, но здесь оператор delete «спрятан» внутри указателя std :: auto_ptr. Это также приводит к неопределенному поведению.
Чтобы исправить это, вы должны указать, что здесь необходимо использовать delete [].
Правильный вариант кода:
std::unique_ptr<char[]> swf(new char[swfSize]);
V599 Деструктор не был объявлен как виртуальный, хотя класс TTileSet содержит виртуальные функции. cellselection.cpp 891
void redo() const
{
insertLevelAndFrameIfNeeded();
TTileSet *tiles; // <=
bool isLevelCreated;
pasteRasterImageInCellWithoutUndo(...., &tiles, ....);
delete tiles; // <=
TApp::instance()->getCurrentXsheet()->notifyXsheetChanged();
}
Теперь поговорим об утечках памяти и частичном разрушении объектов. В этом примере объекты, унаследованные от класса TTileSet, не будут полностью уничтожены.
Описание класса TTileSet:
class DVAPI TTileSet
{
....
protected:
TDimension m_srcImageSize;
typedef std::vector<Tile *> Tiles;
Tiles m_tiles;
public:
TTileSet(const TDimension &dim) : m_srcImageSize(dim)
{
}
~TTileSet(); // <=
....
virtual void add(const TRasterP &ras, TRect rect) = 0;
....
virtual TTileSet *clone() const = 0;
};
Класс абстрактный и содержит чистые виртуальные функции. Вы не можете создавать объекты этого класса, поскольку он используется только производными классами. Таким образом, из-за отсутствия виртуального деструктора в TTileSet (деструктор есть, но он не помечен как виртуальный), все производные классы не могут быть полностью очищены.
В коде OpenToonz я обнаружил несколько классов, унаследованных от TTileSet:
class DVAPI TTileSetCM32 : public TTileSet class DVAPI TTileSetCM32 : public TTileSet class DVAPI TTileSetFullColor : public TTileSet class DVAPI Tile : public TTileSet::Tile
Каждый из этих классов объектов (или производных от них) не будет уничтожен полностью. Вероятный результат - неопределенное поведение; на практике это может привести к утечкам памяти и другим ресурсам.
Разработчикам также стоит ознакомиться со следующими фрагментами:
- V599 Виртуальный деструктор отсутствует, хотя класс MessageParser содержит виртуальные функции. tipcsrv.cpp 91
- V599 Виртуальный деструктор отсутствует, хотя класс ColumnToCurveMapper содержит виртуальные функции. functionselection.cpp 278
Опасное использование указателей
V503 Это бессмысленное сравнение: указатель ‹0. styleselection.cpp 104
bool pasteStylesDataWithoutUndo(....)
{
....
if (palette->getStylePage(styleId) < 0) { // <=
// styleId non e' utilizzato: uso quello
// (cut/paste utilizzato per spostare stili)
palette->setStyle(styleId, style);
} else {
// styleId e' gia' utilizzato. ne devo prendere un altro
styleId = palette->getFirstUnpagedStyle();
if (styleId >= 0)
palette->setStyle(styleId, style);
else
styleId = palette->addStyle(style);
}
....
}
Функция getStylePage () возвращает указатель на некоторую страницу: TPalette :: Page *. Такое сравнение с 0 не имеет смысла. Я исследовал способ использования функции getStylePage () и увидел, что во всех других случаях результат этой функции проверяется на нулевое значение, но здесь программист допустил ошибку.
V522 Может иметь место разыменование нулевого указателя региона. Проверить логическое условие. palettecmd.cpp 102
bool isStyleUsed(const TVectorImageP vi, int styleId)
{
....
TRegion *region = vi->getRegion(i);
if (region || region->getStyle() != styleId)
return true;
....
}
Скорее всего, программист поставил операторы ’&&’ и ‘||’ в неправильные места. В противном случае, если указатель region пуст, он будет разыменован.
V614 Используется потенциально неинициализированный указатель socket. Рассмотрите возможность проверки первого фактического аргумента функции connect. tmsgcore.cpp 36
void TMsgCore::OnNewConnection() //server side
{
QTcpSocket *socket;
if (m_tcpServer) // <=
socket = m_tcpServer->nextPendingConnection(); // <=
assert(socket);
bool ret = connect(socket, ....); // <=
ret = ret && connect(socket, ....); // <=
assert(ret);
m_sockets.insert(socket);
}
Анализатор обнаружил возможное использование неинициализированного указателя socket. Если переменная m_tcpServer имеет значение false, указатель не будет инициализирован. Но, будучи неинициализированным, его все же можно передать в функцию connect ().
V595 Указатель batchesTask использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 1064, 1066. batches.cpp 1064
void BatchesController::update()
{
....
TFarmTask *batchesTask = getTask(batchesTaskId); // <=
TFarmTask farmTask = *batchesTask; // <=
if (batchesTask) { // <=
QString batchesTaskParentId = batchesTask->m_parentId;
m_controller->queryTaskInfo(farmTaskId, farmTask);
int chunkSize = batchesTask->m_chunkSize;
*batchesTask = farmTask;
batchesTask->m_chunkSize = chunkSize;
batchesTask->m_id = batchesTaskId;
batchesTask->m_parentId = batchesTaskParentId;
}
....
}
Есть много фрагментов, где потенциально может произойти разыменование нулевого указателя. Обычно есть необходимая проверка, но один или несколько фрагментов по-прежнему небезопасны. Например, есть проверка batchesTask,, но указатель уже разыменован перед проверкой.
29 похожих фрагментов показаны здесь, в файле: OpenToonz_V595.txt
Ошибки, связанные с работой со строками
V530 Необходимо использовать возвращаемое значение функции toUpper. sceneviewerevents.cpp 847
void SceneViewer::keyPressEvent(QKeyEvent *event)
{
....
QString text = event->text();
if ((event->modifiers() & Qt::ShiftModifier))
text.toUpper();
....
}
Метод ToUpper () не изменяет строку «текст». В документации он описан как: QString QString :: toUpper (), т.е. это постоянный метод.
Правильный вариант кода:
QString text = event->text();
if ((event->modifiers() & Qt::ShiftModifier))
text = text.toUpper();
В коде есть три функции, возвращаемое значение которых не используется. Все эти фрагменты нужно отредактировать:
- V530 Необходимо использовать возвращаемое значение функции «left». tfarmserver.cpp 569
- V530 Необходимо использовать возвращаемое значение функции «ftell». tiio_bmp.cpp 804
- V530 Необходимо использовать возвращаемое значение функции "накопление". bendertool.cpp 374
V614 Использован неинициализированный итератор it1. fxcommand.cpp 2096
QString DeleteLinksUndo::getHistoryString()
{
....
std::list<TFxP>::const_iterator it1; // <=
std::list<TFx *>::const_iterator ft;
for (ft = m_terminalFxs.begin(); ft != ....end(); ++ft) {
if (ft != m_terminalFxs.begin())
str += QString(", ");
str += QString("%1- -Xsheet")
.arg(QString::fromStdWString((*it1)->getName())); // <=
}
....
}
Неинициализированный итератор it1 используется в строковых операциях. Скорее всего, программист забыл заменить его на итератор ft.
V642 Сохранять результат функции _wcsicmp внутри переменной типа char неуместно. Значимые биты могут быть потеряны, нарушив логику программы. tfilepath.cpp 328
bool TFilePath::operator<(const TFilePath &fp) const
{
....
char differ;
differ = _wcsicmp(iName.c_str(), jName.c_str());
if (differ != 0)
return differ < 0 ? true : false;
....
}
Функция _wcsicmp возвращает следующие значения типа int:
- ‹0 - строка1 меньше, чем строка2;
- 0 - строка1 идентично строке2;
- ›0 - строка1 больше, чем строка2.
Обратите внимание, что «› 0 »может быть любым числом, а не только 1. Эти числа могут быть: 2, 3, 100, 256, 1024, 5555 и так далее. Результат функции _wcsicmp может не соответствовать переменной типа char, поэтому оператор сравнения вернет неожиданный результат.
V643 Необычная арифметика указателя: \\ + v [i]. К указателю строки добавляется значение типа char. tstream.cpp 31
string escape(string v)
{
int i = 0;
for (;;) {
i = v.find_first_of("\\\'\"", i);
if (i == (int)string::npos)
break;
string h = "\\" + v[i]; // <=
v.insert(i, "\\");
i = i + 2;
}
return v;
}
Анализатор обнаружил ошибку, вызванную добавлением символьной константы к строковому литералу. Ожидалось, что к строке будет добавлен символ, но к указателю на строку добавляется числовое значение, что приводит к доступу за границу строкового литерала и неожиданному результату.
Вот чему равен этот код:
const char *p1 = "\\"; const int delta = v[i]; const char *p2 = *p1 + delta; string h = p2;
Правильный вариант кода:
string h = string("\\") + v[i];
V655 Строки были объединены, но не используются. Рассмотрите возможность проверки выражения псевдоним +«] ». plasticdeformerfx.cpp 150
string PlasticDeformerFx::getAlias(....) const
{
std::string alias(getFxType());
alias += "[";
....
if (sd)
alias += ", "+toString(sd, meshColumnObj->paramsTime(frame));
alias + "]"; // <=
return alias;
}
Анализатор обнаружил выражение, результат которого не используется. Скорее всего, оператор «+» был случайно написан вместо «+ =». В результате квадратная скобка не добавляется к строке псевдонима, как планировал программист.
Неправильные исключения
V596 Объект создан, но не используется. Ключевое слово throw могло отсутствовать: throw domain_error (FOO); pluginhost.cpp 1486
void Loader::doLoad(const QString &file)
{
....
int ret = pi->ini_(host);
if (ret) {
delete host;
std::domain_error("failed initialized: error on ....");
}
....
}
Ключевое слово throw было случайно забыто в функции. В результате этот код не генерирует исключение в случае возникновения ошибки. Правильный вариант кода:
throw std::domain_error("failed initialized: error on ....");
V746 Нарезка шрифтов. Исключение следует перехватывать по ссылке, а не по значению. iocommand.cpp 1620
bool IoCmd::saveLevel(....)
{
....
try {
sl->save(fp, TFilePath(), overwritePalette);
} catch (TSystemException se) { // <=
QApplication::restoreOverrideCursor();
MsgBox(WARNING, QString::fromStdWString(se.getMessage()));
return false;
} catch (...) {
....
}
....
}
Анализатор обнаружил потенциальную ошибку, связанную с отловом исключения по значению. Это означает, что новый объект se для TSystemException будет создан с помощью конструктора копирования. В то же время код потеряет некоторую информацию об исключении, которое было сохранено в классах, унаследованных от TSystemException.
Подобные подозрительные фрагменты:
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. iocommand.cpp 2650
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. projectpopup.cpp 522
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. projectpopup.cpp 537
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. projectpopup.cpp 635
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. tlevel_io.cpp 130
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. calligraph.cpp 161
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. calligraph.cpp 165
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. patternmap.cpp 210
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. patternmap.cpp 214
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. patternmap.cpp 218
- V746 Нарезка типов. Исключение следует перехватывать по ссылке, а не по значению. scriptbinding_level.cpp 221
Неправильные условия
V547 Выражение ‘(int) startOutPoints.size ()% 2! = 2’ всегда истинно. rasterselection.cpp 852
TStroke getIntersectedStroke(TStroke &stroke, TRectD bbox)
{
....
for (t = 0; t < (int)outPoints.size(); t++)
addPointToVector(...., (int)startOutPoints.size() % 2 != 2);
....
}
Интересный баг. Возможно, программист хотел проверить, является ли значение size () четным или нечетным . Вот почему остаток от деления на 2 нужно сравнивать с нулем.
V502 Возможно, оператор ?: Работает не так, как ожидалось. Оператор ?: Имеет более низкий приоритет, чем оператор +. igs_motion_wind_pixel.cpp 127
void rgb_to_lightness_(
const double re, const double gr, const double bl, double &li)
{
li=((re < gr) ? ((gr < bl) ? bl : gr) : ((re < bl) ? bl : re) +
(gr < re)
? ((bl < gr) ? bl : gr)
: ((bl < re) ? bl : re)) / 2.0;
}
В этом фрагменте кода программист допустил ошибку, связанную с приоритетом тернарного оператора ‘:?’. Его приоритет ниже, чем у оператора сложения. Следовательно, если условие (re ‹gr) ложно, следующие вычисления будут выполнены неправильно: реальные переменные будут добавлены к логическим.
Никогда не используйте сразу несколько тернарных операторов - это самый простой способ ошибиться.
V590 Рассмотрите возможность проверки состояния ‘state == (- 3) || state! = 0 ’выражение. Выражение является чрезмерным или содержит опечатку. psdutils.cpp 174
int psdUnzipWithoutPrediction(....)
{
....
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);
....
}
Условие, отмеченное стрелкой, не зависит от результата подвыражения «state == Z_DATA_ERROR». Это легко проверить, если вы построите таблицу истинности всего условного выражения.
Копирование и вставка программирования
V517 Использование шаблона if (A) {…} else if (A) {…}. Есть вероятность наличия логической ошибки. Проверить строки: 1448, 1454. tcenterlineskeletonizer.cpp 1448
inline void Event::processVertexEvent()
{
....
if (newLeftNode->m_concave) { // <=
newLeftNode->m_notOpposites = m_generator->m_notOpposites;
append<vector<ContourEdge *>, vector<ContourEdge *>::....
newLeftNode->m_notOpposites.push_back(newRightNode->m_edge);
newLeftNode->m_notOpposites.push_back(newRightNode->....);
} else if (newLeftNode->m_concave) { // <=
newRightNode->m_notOpposites = m_generator->m_notOpposites;
append<vector<ContourEdge *>, vector<ContourEdge *>::....
newRightNode->m_notOpposites.push_back(newLeftNode->m_edge);
newRightNode->m_notOpposites.push_back(newLeftNode->....);
}
....
}
Мы видим, что переменные newLeftNode и newRightNode перепутаны в условиях. В результате этой ошибки ветвь else никогда не выполняется. Скорее всего, одно из условий должно быть таким: if (newRightNode- ›m_concave).
V501 Слева и справа от оператора ‘||’ находятся идентичные подвыражения: m_cutLx || m_cutLx canvassizepopup.cpp 271
bool m_cutLx, m_cutLy;
void PeggingWidget::on00()
{
....
m_11->setIcon(...).rotate(m_cutLx || m_cutLx ? -90 : 90),....));
....
}
В коде есть две логические переменные: m_cutLx и m_cutLy, которые отличаются только одной буквой. Но в приведенном примере мы видим, что используется только m_cutLx. Возможно, в одном из них допущена опечатка.
V501 Слева и справа от оператора || есть идентичные подвыражения parentTask-› m_status == Aborted . tfarmcontroller.cpp 1857
void FarmController::taskSubmissionError(....)
{
....
if (parentTask->m_status == Aborted || // <=
parentTask->m_status == Aborted) { // <=
parentTask->m_completionDate = task->m_completionDate;
if (parentTask->m_toBeDeleted)
m_tasks.erase(itParent);
}
....
}
Анализатор обнаружил два похожих сравнения с константой Прервано. Произведя поиск в файле, я нашел аналогичный блок кода в строке 2028 с таким условием:
if (parentTask->m_status == Completed ||
parentTask->m_status == Aborted) {
Возможно, в этом фрагменте условие должно быть аналогичным.
V501 Слева и справа от оператора || есть идентичные подвыражения cornerCoords.y› upperBound . tellipticbrush.cpp 1020
template <typename T>
void tellipticbrush::OutlineBuilder::addMiterSideCaps(....)
{
....
if (cornerCoords == TConsts::napd ||
cornerCoords.x < lowerBound || cornerCoords.y > upperBound ||
cornerCoords.y < lowerBound || cornerCoords.y > upperBound) {
....
}
....
}
Здесь программист допустил небольшую опечатку, используя y вместо x.
Я не буду описывать еще шесть опечаток, вызванных копипастом, просто приведу их в виде списка. Эти фрагменты также должны быть обязательно просмотрены разработчиками:
- V501 Слева и справа от оператора ‘||’ есть идентичные подвыражения ‘s.m_repoStatus ==« modified ». svnupdatedialog.cpp 210
- V501 Слева и справа от оператора ‘||’ есть идентичные подвыражения ‘m_lineEdit-› hasFocus () ’. framenavigator.cpp 44
- V517 Обнаружено использование шаблона «if (A) {…} else if (A) {…}». Есть вероятность наличия логической ошибки. Проверить строки: 750, 825. tpalette.cpp 750
- V517 Обнаружено использование шаблона «if (A) {…} else if (A) {…}». Есть вероятность наличия логической ошибки. Проверить строки: 123, 126. igs_de density.cpp 123
- V523 Оператор «then» эквивалентен оператору «else». typetool.cpp 813
- V583 Оператор «?:», Независимо от его условного выражения, всегда возвращает одно и то же значение: запятую. tgrammar.cpp 731
Разные ошибки
V665 Возможно, использование #pragma warning (по умолчанию: X) в данном контексте неверно. Вместо этого следует использовать #pragma warning (push / pop). Проверить строки: 20, 205. tspectrum.h 205
#ifdef WIN32 #pragma warning(disable : 4251) #endif .... #ifdef WIN32 #pragma warning(default : 4251) #endif
Вот как компилятор отключает предупреждения, которые наконец-то были замечены в этом проекте. Ошибка заключается в том, что предупреждение #pragma (по умолчанию: X) не включает предупреждение, а устанавливает его как ПО УМОЛЧАНИЮ, которое может отличаться от того, что ожидает программист. Правильный вариант кода должен быть:
#ifdef WIN32 #pragma warning(push) #pragma warning(disable : 4251) #endif .... #ifdef WIN32 #pragma warning(pop) #endif
V546 Член класса инициализируется сам по себе: m_subId (m_subId). tfarmcontroller.cpp 572
class TaskId
{
int m_id;
int m_subId;
public:
TaskId(int id, int subId = -1) : m_id(id), m_subId(m_subId){};
Интересный баг в списке инициализации класса. Поле m_subld инициализируется само по себе; возможно, программист хотел написать m_subId (subId).
V557 Возможно переполнение массива. Индекс 9 указывает за пределы массива. tconvolve.cpp 123
template <class PIXOUT>
void doConvolve_cm32_row_9_i(....)
{
TPixel32 val[9]; // <=
....
for (int i = 0; i < 9; ++i) { // <= OK
....
else if (tone == 0)
val[i] = inks[ink];
else
val[i] = blend(....);
}
pixout->r = (typename PIXOUT::Channel)((
val[1].r * w1 + val[2].r * w2 + val[3].r * w3 +
val[4].r * w4 + val[5].r * w5 + val[6].r * w6 +
val[7].r * w7 + val[8].r * w8 + val[9].r * w9 + // <= ERR
(1 << 15)) >> 16);
pixout->g = (typename PIXOUT::Channel)((
val[1].g * w1 + val[2].g * w2 + val[3].g * w3 +
val[4].g * w4 + val[5].g * w5 + val[6].g * w6 +
val[7].g * w7 + val[8].g * w8 + val[9].g * w9 + // <= ERR
(1 << 15)) >> 16);
pixout->b = (typename PIXOUT::Channel)((
val[1].b * w1 + val[2].b * w2 + val[3].b * w3 +
val[4].b * w4 + val[5].b * w5 + val[6].b * w6 +
val[7].b * w7 + val[8].b * w8 + val[9].b * w9 + // <= ERR
(1 << 15)) >> 16);
pixout->m = (typename PIXOUT::Channel)((
val[1].m * w1 + val[2].m * w2 + val[3].m * w3 +
val[4].m * w4 + val[5].m * w5 + val[6].m * w6 +
val[7].m * w7 + val[8].m * w8 + val[9].m * w9 + // <= ERR
(1 << 15)) >> 16);
....
}
Это большой фрагмент кода, где программист обращается к массиву val, состоящему из 9 элементов, по индексу от 1 до 9. Хотя есть цикл, в котором мы видим правильный доступ к массиву со стороны индекс от 0 до 8.
V556 Сравниваются значения разных типов перечислений: m_action! = EDIT_SEGMENT. Типы: Action, CursorType. controlpointeditortool.cpp 257
enum Action { NONE,
RECT_SELECTION,
CP_MOVEMENT,
SEGMENT_MOVEMENT,
IN_SPEED_MOVEMENT,
OUT_SPEED_MOVEMENT };
enum CursorType { NORMAL,
ADD,
EDIT_SPEED,
EDIT_SEGMENT,
NO_ACTIVE };
void ControlPointEditorTool::drawMovingSegment()
{
int beforeIndex = m_moveSegmentLimitation.first;
int nextIndex = m_moveSegmentLimitation.second;
if (m_action != EDIT_SEGMENT || // <=
beforeIndex == -1 ||
nextIndex == -1 ||
!m_moveControlPointEditorStroke.getStroke())
return;
....
}
Анализатор обнаружил сравнение значений enum разных типов. Используя поиск кода, я также обнаружил, что поле класса m_action инициализировано с правильным типом, но в этом фрагменте оно сравнивается с константой другого типа.
Вывод
Как уже было сказано, проект OpenToonz - отличная находка для статического анализатора кода: даже будучи довольно маленьким, он содержит множество серьезных ошибок. В этой статье перечислены не все ошибки; более того, мы не смогли уместить некоторые серьезные предупреждения из-за их большого количества. Сообщим разработчикам об обнаруженных ошибках, возможно, они будут заинтересованы в улучшении своего кода.
Компания Pixar также выразила намерение открыть исходный код Universal Scene Description (USD). Мы с нетерпением ждем этого.
Для тех, кому может быть интересно: вы можете найти здесь PVS-Studio и запустить его на своих C / C ++ / C # проектах. Анализатор работает в среде Windows и поддерживает различные системы сборки.
