# Повторная проверка проекта Notepad\+\+ Прошло более года, как мы проверили Notepad\+\+ с помощью PVS\-Studio\. Интересно посмотреть, насколько анализатор PVS\-Studio стал лучше, и что было исправлено в Notepad\+\+ из прежних ошибок\. ## Введение Итак, мы проверили проект Notepad\+\+ взятый из репозитория 31 января 2012\. Для проверки использовался анализатор [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) версии 4\.54\. ![0131_NotepadPP2_ru/image1.png](https://import.viva64.com/docx/blog/0131_NotepadPP2_ru/image1.png) Как уже было сказано, мы ранее проверяли этот проект\. Ошибок нашли не много, но всё\-таки что\-то нашли\. В новой версии проекта часть старых ошибок исправлена, а часть нет\. Это странно\. По всей видимости, прежняя заметка осталась незамеченной авторами Notepad\+\+ и они не воспользовались PVS\-Studio для проверки проекта\. Возможно эта заметка все\-таки привлечет авторов Notepad\+\+, тем более что мы недавно [изменили](https://pvs-studio.ru/ru/blog/posts/0128/) режим триала и все найденные с помощью PVS\-Studio ошибки видны\. Исправленные ошибки, видимо были найдены другими методами тестирования или о них сообщили пользователи\. Например, исправлена ошибка, описанная в предыдущей статье, которая касалась заполнения массива \_iContMap\. Было так: ```cpp memset(_iContMap, -1, CONT_MAP_MAX); ``` Исправленный вариант: ```cpp memset(_iContMap, -1, CONT_MAP_MAX * sizeof(int)); ``` Зато вот эта ошибка, продолжает жить и здравствовать: ```cpp bool isPointValid() { return _isPointXValid && _isPointXValid; }; ``` Диагностическое сообщение анализатора PVS\-Studio: V501 There are identical sub\-expressions to the left and to the right of the '&&' operator: \_isPointXValid && \_isPointXValid Notepad\+\+ parameters\.h 166 Мы не будем возвращаться к тем ошибкам, которые были описаны в предыдущей статье\. Рассмотрим то новое, что научился диагностировать анализатор PVS\-Studio за прошедшее время: Как всегда хотим напомнить, что это далеко не все найденные недочёты, а то только те, которые показались нам интересными для написания статьи\. Не забывайте про новый режим триала, который как нельзя лучше подходит для проверки в том числе и opensource\-проектов\. ## Новые найденные ошибки ### Ошибка N1\. Выход за границы массива ```cpp int encodings[] = { 1250, 1251, 1252, .... }; BOOL CALLBACK DefaultNewDocDlg::run_dlgProc( UINT Message, WPARAM wParam, LPARAM) { ... for (int i = 0 ; i <= sizeof(encodings)/sizeof(int) ; i++) { int cmdID = em->getIndexFromEncoding(encodings[i]); ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V557 Array overrun is possible\. The value of 'i' index could reach 46\. Notepad\+\+ preferencedlg\.cpp 984 В цикле перебираются элементы массива "encodings"\. Ошибка заключается в неправильном условии остановки цикла\. При последней итерации цикла происходит доступ к памяти за границей массива\. Ошибку можно исправить, заменив условие "<\=" на "<"\. Корректный код: ```cpp for (int i = 0 ; i < sizeof(encodings)/sizeof(int) ; i++) ``` ### Ошибка N2\. Неправильное вычисление размера буфера ```cpp typedef struct tagTVITEMA { ... LPSTR pszText; ... } TVITEMA, *LPTVITEMA; #define TVITEM TVITEMA HTREEITEM TreeView::addItem(...) { TVITEM tvi; ... tvi.cchTextMax = sizeof(tvi.pszText)/sizeof(tvi.pszText[0]); ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V514 Dividing sizeof a pointer 'sizeof \(tvi\.pszText\)' by another value\. There is a probability of logical error presence\. Notepad\+\+ treeview\.cpp 88 Размер буфера вычисляется с помощью выражения "sizeof\(tvi\.pszText\)/sizeof\(tvi\.pszText\[0\]\)"\. Это выражение не имеет смысла\. В нём размер указателя делится на размер одного символа\. Как исправить код сказать сложно, так как мы не знакомы с логикой работы программы\. ### Ошибка N3\. Неправильная проверка, что строка пустая ```cpp size_t Printer::doPrint(bool justDoIt) { ... TCHAR headerM[headerSize] = TEXT(""); ... if (headerM != '\0') ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V528 It is odd that pointer to 'char' type is compared with the '\\0' value\. Probably meant: \*headerM \!\= '\\0'\. Notepad\+\+ printer\.cpp 380 Указатель сравнивается с нулевым значением\. Ноль задан литералом вида '\\0'\. Это подсказывает нам, что здесь забыто разыменование указателя\. Корректный код: ```cpp if (*headerM != '\0') ``` Аналогичная ошибка допущена в другом месте: V528 It is odd that pointer to 'char' type is compared with the '\\0' value\. Probably meant: \*headerR \!\= '\\0'\. Notepad\+\+ printer\.cpp 392 ### Ошибка N4\. Опечатка в условии ```cpp DWORD WINAPI Notepad_plus::threadTextPlayer(void *params) { ... const char *text2display = ...; ... if (text2display[i] == ' ' && text2display[i] == '.') ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V547 Expression is always false\. Probably the '\|\|' operator should be used here\. Notepad\+\+ notepad\_plus\.cpp 4967 Условие \(text2display\[i\] \=\= ' ' && text2display\[i\] \=\= '\.'\) никогда не выполняется\. Символ не может одновременно являться и пробелом и точкой\. Видимо здесь мы имеем дело с простой опечаткой и код должен был выглядеть следующим образом: ```cpp if (text2display[i] == ' ' || text2display[i] == '.') ``` Аналогичная ошибка допущена в другом месте: V547 Expression is always false\. Probably the '\|\|' operator should be used here\. Notepad\+\+ notepad\_plus\.cpp 5032 ### Ошибка N5\. Опечатка в условии ```cpp int Notepad_plus::getHtmlXmlEncoding(....) const { ... if (langT != L_XML && langT != L_HTML && langT == L_PHP) return -1; ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V590 Consider inspecting this expression\. The expression is excessive or contains a misprint\. Notepad\+\+ notepad\_plus\.cpp 853 Имеющуюся в коде проверку можно упростить\. Тогда код будет выглядеть следующим образом: ```cpp if (langT == L_PHP) ``` Это явно не то, что хотел программист\. Видимо здесь мы опять имеем дело с опечаткой\. Корректный код: ```cpp if (langT != L_XML && langT != L_HTML && langT != L_PHP) ``` ### Ошибка N6\. Неправильная работа с битами ```cpp TCHAR GetASCII(WPARAM wParam, LPARAM lParam) { ... result=ToAscii(wParam,(lParam >> 16) && 0xff, keys,&dwReturnedValue,0); ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V560 A part of conditional expression is always true: 0xff\. Notepad\+\+ babygrid\.cpp 694 В коде хотят извлечь третий байт из переменной 'lParam'\. Из\-за опечатки, код делает совсем не это\. Ошибка заключается в том, что используется оператор '&&' вместо '&'\. Корректный код: ```cpp result=ToAscii(wParam,(lParam >> 16) & 0xff, keys,&dwReturnedValue,0); ``` ### Ошибка N7\. Недописанный код ```cpp #define SCE_T3_BRACE 20 static inline bool IsAnOperator(const int style) { return style == SCE_T3_OPERATOR || SCE_T3_BRACE; } ``` Диагностическое сообщение анализатора PVS\-Studio: V560 A part of conditional expression is always true: 20\. lextads3\.cxx 700 Функция IsAnOperator\(\) всегда возвращает 'true'\. Корректный код: ```cpp return style == SCE_T3_OPERATOR || style == SCE_T3_BRACE; ``` ### Ошибка N8\. Код, который никогда не будет выполнен ```cpp static void ColouriseVHDLDoc(....) { ... } else if (sc.Match('-', '-')) { sc.SetState(SCE_VHDL_COMMENT); sc.Forward(); } else if (sc.Match('-', '-')) { if (sc.Match("--!")) sc.SetState(SCE_VHDL_COMMENTLINEBANG); else sc.SetState(SCE_VHDL_COMMENT); } ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V517 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 130, 133\. lexvhdl\.cxx 130 Если первое условие \(sc\.Match\('\-', '\-'\)\) истинно, то вторая проверка выполнена не будет\. Это приведет к тому, что случай последовательность символов "\-\-\!" никогда не будет обработана правильно\. По всей видимости, здесь часть кода лишняя и должно быть написано так: ```cpp static void ColouriseVHDLDoc(....) { ... } else if (sc.Match('-', '-')) { if (sc.Match("--!")) sc.SetState(SCE_VHDL_COMMENTLINEBANG); else sc.SetState(SCE_VHDL_COMMENT); } ... } ``` ### Ошибка N9\. Лишний код Есть фрагменты кода, которые не приводят к проблемам, но являются избыточными\. Приведем два примера такого типа: ```cpp void Gripper::doTabReordering(POINT pt) { ... else if (_hTab == hTabOld) { /* delete item on switch between tabs */ ::SendMessage(_hTab, TCM_DELETEITEM, iItemOld, 0); } else { if (_hTab == hTabOld) { /* delete item on switch between tabs */ ::SendMessage(_hTab, TCM_DELETEITEM, iItemOld, 0); } } ... } ``` Диагностическое сообщение анализатора PVS\-Studio: V571 Recurring check\. The 'if \(\_hTab \=\= hTabOld\)' condition was already verified in line 478\. Notepad\+\+ gripper\.cpp 485 Вторая часть кода не имеет никакого смысла и может быть удалена\. А вот другой пример, где присутствуют лишние проверк\. Указатели 'mainVerStr' и 'auxVerStr' всегда не равны нулю, так как это массивы, созданные на стеке: ```cpp LRESULT Notepad_plus::process(....) { ... TCHAR mainVerStr[16]; TCHAR auxVerStr[16]; ... if (mainVerStr) mainVer = generic_atoi(mainVerStr); if (auxVerStr) auxVer = generic_atoi(auxVerStr); ... } Здесь можно написать проще: mainVer = generic_atoi(mainVerStr); auxVer = generic_atoi(auxVerStr); ``` Проверки, показанные выше, встречаются в проекте Notepad\+\+ неоднократно: V600 Consider inspecting the condition\. The 'mainVerStr' pointer is always not equal to NULL\. Notepad\+\+ nppbigswitch\.cpp 938 V600 Consider inspecting the condition\. The 'auxVerStr' pointer is always not equal to NULL\. Notepad\+\+ nppbigswitch\.cpp 940 V600 Consider inspecting the condition\. The 'intStr' pointer is always not equal to NULL\. Notepad\+\+ preferencedlg\.cpp 1871 V600 Consider inspecting the condition\. The 'intStr' pointer is always not equal to NULL\. Notepad\+\+ userdefinedialog\.cpp 222 V600 Consider inspecting the condition\. The 'intStr' pointer is always not equal to NULL\. Notepad\+\+ wordstyledlg\.cpp 539 ## Выводы Статические анализаторы кода, это не те инструменты, которые можно использовать время от времени\. Используя их регулярно можно быстро находить ошибки, тем самым в десятки раз сокращая цену их устранения\. Зачем подолгу искать странное поведение программы, из\-за неочищенного массива? Код "memset\(\_iContMap, \-1, CONT\_MAP\_MAX\)" легко и быстро находится статическим анализатором\. Если эта ошибка найдена всё же статическим анализатором PVS\-Studio, то всё равно инструмент был использован неправильно\. Во\-первых, другие диагностические сообщения были изучены не внимательно\. Во\-вторых, использовать статический анализ нужно регулярно\. Это позволяет быстро устранить ошибки в только что написанном коде\. Плюс в PVS\-Studio постоянно появляются новые диагностические правила\. Ещё раз хочется подчеркнуть мысль, что статический анализ это инструмент для регулярного использования\. Это как расширение списка warnings, выдаваемых компилятором\. Вы работаете с отключенными warnings и включаете их только изредка, когда есть настроение? Нет\. Аналогично нужно относиться и к диагностическим предупреждениям, выдаваемым инструментами статического анализа\. Как это сделать? Можно [запускать](https://pvs-studio.ru/ru/docs/manual/0005/) анализ ночью на сервере\. Можно использовать [инкрементальный анализ](https://pvs-studio.ru/ru/docs/manual/0024/), чтобы проверять только что модифицированные файлы\. Если кажется, что анализ выполняется медленно и отнимает много ресурсов, то предлагаем ознакомиться с [советами по повышению скорости работы](https://pvs-studio.ru/ru/docs/manual/0023/)\.