# PVS\-Studio vs Chromium \- продолжение Приблизительно полгода назад мы проверили проект Chromium и написали про это статью\. Естественно, анализатор PVS\-Studio постоянно развивается, и в новой версии Chromium мы нашли некоторые необнаруженные ранее ошибки\. Конечно, многие ошибки относятся не к самому проекту Chromium, а к используемым в нём библиотекам\. Однако в статье я хочу показать, что улучшились возможности анализатора, а не то, в какой части Chromium что\-то нашлось, а в какой нет\. Поэтому сообщения будут приведены вместе\. Итак, в этой заметке мы перечислим то новое, что нашел анализатор\. Это далеко не всё, так как отчет мы просмотрели быстро и выписали только те фрагменты, на которых остановился взгляд\. Для более подробного анализа Chromium или библиотек мы можем предоставить на время разработчикам полноценную версию PVS\-Studio\. Кстати, перейдите по [этой ссылке](https://pvs-studio.ru/ru/blog/posts/0092/) и, возможно, Вы тоже заинтересуетесь возможностью полноценно попробовать PVS\-Studio\. **Фрагмент N1** ```cpp std::string TestFileIO::TestParallelReads() { ... const char* expected_result_1 = "__border__abc__border__"; const char* expected_result_2 = "__border__defghijkl__border__"; if (strncmp(extended_buf_1, expected_result_1, sizeof(expected_result_1)) != 0 || strncmp(extended_buf_2, expected_result_2, sizeof(expected_result_2)) != 0) { ... } ``` Диагностические сообщения PVS\-Studio: V579 The strncmp function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. ppapi\_tests test\_file\_io\.cc 759 V579 The strncmp function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. ppapi\_tests test\_file\_io\.cc 761 Имеющиеся в коде вызовы функции strncmp\(\) сравнивают только первые символы, а не строки целиком\. Для вычисления длины строк попытались использовать совершенно неуместный здесь оператор sizeof\(\)\. Оператор sizeof\(\) вычислит размер указателя, а вовсе не количество байт в строке\. **Фрагмент N2** ```cpp int AffixMgr::parse_convtable(..., const char * keyword) { ... if (strncmp(piece, keyword, sizeof(keyword)) != 0) { ... } ``` Диагностическое сообщение PVS\-Studio: V579 The strncmp function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. hunspell affixmgr\.cxx 3545 Вновь ошибка в том, что будут сравниваться только первые 4 или 8 байт, в зависимости от размера указателя\. **Фрагмент N3** ```cpp #define SEC_ASN1_CHOICE 0x100000 typedef struct sec_ASN1Template_struct { unsigned long kind; ... } SEC_ASN1Template; PRBool SEC_ASN1IsTemplateSimple( const SEC_ASN1Template *theTemplate) { ... if (!theTemplate->kind & SEC_ASN1_CHOICE) { ... } ``` Диагностическое сообщение PVS\-Studio: V564 The '&' operator is applied to bool type value\. You've probably forgotten to include parentheses or intended to use the '&&' operator\. nss secasn1u\.c 121 Ошибка связана с приоритетом операций\. Корректный вариант кода: ```cpp if (!(theTemplate->kind & SEC_ASN1_CHOICE)) { ``` **Фрагмент N4** ```cpp bool GetPlatformFileInfo(...) { ... info->is_directory = file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0; ... } ``` Диагностическое сообщение PVS\-Studio: V564 The '&' operator is applied to bool type value\. You've probably forgotten to include parentheses or intended to use the '&&' operator\. base platform\_file\_win\.cc 219 Ошибка связана с приоритетом операций\. Корректный вариант кода: ```cpp info->is_directory = (file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; ``` **Фрагмент N5** ```cpp WebRtc_Word32 interpolator::SupportedVideoType(VideoType srcVideoType, VideoType dstVideoType) { ... if ((srcVideoType != kI420) || (srcVideoType != kIYUV) || (srcVideoType != kYV12)) { return -1; } ... } ``` Диагностическое сообщение PVS\-Studio: V547 Expression is always true\. Probably the '&&' operator should be used here\. webrtc\_vplib interpolator\.cc 119 Условие вида \(A \!\= 123 \|\| A\!\= 321\) всегда истинно\. Очевидно, здесь есть опечатка, и условие должно было выглядеть по\-другому\. **Фрагмент N6** ```cpp static GLenum get_temp_image_type(GLcontext *ctx, GLenum baseFormat) { ... if (ctx->DrawBuffer->Visual.redBits <= 8) return GL_UNSIGNED_BYTE; else if (ctx->DrawBuffer->Visual.redBits <= 8) return GL_UNSIGNED_SHORT; else return GL_FLOAT; ... } ``` Диагностическое сообщение PVS\-Studio: V517 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 2520, 2522\. osmesa meta\.c 2520 Два раза выполняется одинаковая проверка\. Видимо, должно быть: ```cpp if (ctx->DrawBuffer->Visual.redBits <= 8) return GL_UNSIGNED_BYTE; else if (ctx->DrawBuffer->Visual.redBits <= 16) return GL_UNSIGNED_SHORT; ``` **Фрагмент N7** ```cpp WebRtc_Word32 ModuleFileUtility::UpdateWavHeader(OutStream& wav) { ... if(STR_CASE_CMP(codec_info_.plname, "L16") == 0) { res = WriteWavHeader(wav, codec_info_.plfreq, 2, channels, kWaveFormatPcm, _bytesWritten); } else if(STR_CASE_CMP(codec_info_.plname, "PCMU") == 0) { res = WriteWavHeader(wav, 8000, 1, channels, kWaveFormatMuLaw, _bytesWritten); } else if(STR_CASE_CMP(codec_info_.plname, "PCMU") == 0) { res = WriteWavHeader(wav, 8000, 1, channels, kWaveFormatALaw, _bytesWritten); } else { ... } ``` Диагностическое сообщение PVS\-Studio: V517 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 1324, 1327\. media\_file media\_file\_utility\.cc 1324 Два раза подряд член 'plname' сравнивается со строкой "PCMU"\. Скорее всего, второй раз должна быть использована другая строка\. **Фрагмент N8** ```cpp enum ContentSettingsType; struct EntryMapKey { ContentSettingsType content_type; ... }; bool OriginIdentifierValueMap::EntryMapKey::operator<( const OriginIdentifierValueMap::EntryMapKey& other) const { if (content_type < other.content_type) return true; else if (other.content_type > content_type) return false; return (resource_identifier < other.resource_identifier); } ``` Диагностическое сообщение PVS\-Studio: V517 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 61, 63\. browser content\_settings\_origin\_identifier\_value\_map\.cc 61 Первый раз условие написано так: "A < B"\. А второй раз так: "B \> A"\. Таким образом, вторая проверка не имеет смысла\. Очевидно, в коде имеется опечатка\. **Фрагмент N9** WebRtc\_Word32 ```cpp RTPReceiverVideo::ReceiveH263Codec(...) { ... if (IP_PACKET_SIZE < parsedPacket.info.H263.dataLength + parsedPacket.info.H263.insert2byteStartCode? 2:0) ... } ``` Диагностическое сообщение PVS\-Studio: V502 Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\+' operator\. rtp\_rtcp rtp\_receiver\_video\.cc 480 Оператор '?:' имеет более низкий приоритет, чем оператор '\+'\. В результате условие работает не так, как ожидал программист\. Корректное условие должно выглядеть так: ```cpp if (IP_PACKET_SIZE < parsedPacket.info.H263.dataLength + (parsedPacket.info.H263.insert2byteStartCode ? 2:0)) ``` Аналогичная ошибка есть здесь: V502 Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\+' operator\. rtp\_rtcp rtp\_receiver\_video\.cc 504 **Фрагмент N10** ```cpp static int xmlXPathCompOpEvalFirst(...) { ... total += xmlXPathCompOpEvalFirst(...); ... total =+ xmlXPathCompOpEvalFilterFirst(ctxt, op, first); ... } ``` Диагностическое сообщение PVS\-Studio: V588 The expression of the 'A \=\+ B' kind is utilized\. Consider reviewing it, as it is possible that 'A \+\= B' was meant\. libxml xpath\.c 12676 Как видно из первой строки, в переменной total считается некоторая сумма\. Однако, потом допущена опечатка и вместо "\+\=", написано "\=\+"\. **Фрагмент N11** ```cpp static VisiblePosition updateAXLineStartForVisiblePosition(...) { ... tempPosition = startPosition.previous(); if (tempPosition.isNull() || tempPosition.isNull()) break; ... } ``` Диагностическое сообщение PVS\-Studio: V501 There are identical sub\-expressions 'tempPosition\.isNull \(\)' to the left and to the right of the '\|\|' operator\. webcore\_remaining accessibilityobject\.cpp 489 Странный код\. **Фрагмент N12** ```cpp TEST(SharedMemoryTest, MultipleThreads) { ... int threadcounts[] = { 1, kNumThreads }; for (size_t i = 0; i < sizeof(threadcounts) / sizeof(threadcounts); i++) { ... } ``` Диагностическое сообщение PVS\-Studio: V501 There are identical sub\-expressions 'sizeof \(threadcounts\)' to the left and to the right of the '/' operator\. base\_unittests shared\_memory\_unittest\.cc 231 Из\-за ошибки, цикл в функции\-тесте делает только одну итерацию\. Корректный вариант цикла: ```cpp for (size_t i = 0; i < sizeof(threadcounts) / sizeof(*threadcounts); i++) { ``` **Фрагмент N13** ```cpp bool ir_algebraic_visitor::reassociate_constant(...) { if (ir1->operands[0]->type->is_matrix() || ir1->operands[0]->type->is_matrix() || ir2->operands[1]->type->is_matrix() || ir2->operands[1]->type->is_matrix()) return false; } ``` Диагностическое сообщение PVS\-Studio: V501 There are identical sub\-expressions 'ir1\-\>operands \[0\]\-\>type\-\>is\_matrix \(\)' to the left and to the right of the '\|\|' operator\. mesa ir\_algebraic\.cpp 189 Видимо код писался методом Copy\-Paste, и затем индексы были поправлены неправильно\. Сравнение должно выглядеть так: ```cpp if (ir1->operands[0]->type->is_matrix() || ir1->operands[1]->type->is_matrix() || ir2->operands[0]->type->is_matrix() || ir2->operands[1]->type->is_matrix()) ``` **Фрагмент N15** ```cpp #define FRAMESAMPLES_HALF 240 #define FRAMESAMPLES 480 typedef struct { ... WebRtc_Word16 realFFT[FRAMESAMPLES_HALF]; WebRtc_Word16 imagFFT[FRAMESAMPLES_HALF]; } ISACUBSaveEncDataStruct; int WebRtcIsac_EncodeStoredDataUb12(...) { ... for(n = 0; n < FRAMESAMPLES; n++) { realFFT[n] = (WebRtc_Word16) (scale * (float)ISACSavedEnc_obj->realFFT[n] + 0.5f); imagFFT[n] = (WebRtc_Word16) (scale * (float)ISACSavedEnc_obj->imagFFT[n] + 0.5f); } ... } ``` Диагностические сообщения PVS\-Studio: V557 Array overrun is possible\. The value of 'n' index could reach 479\. iSAC encode\.c 1307 V557 Array overrun is possible\. The value of 'n' index could reach 479\. iSAC encode\.c 1308 В цикле происходит выход за границы массивов\. Цикл должен перебирать только FRAMESAMPLES\_HALF элементов\. **Фрагмент N16** ```cpp static int coff_helper_gasflags(...) { ... case 'd': datasect = 1; load = 1; readonly = 0; case 'x': code = 1; load = 1; break; ... } ``` Диагностическое сообщение PVS\-Studio: V519 The 'load' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1346, 1350\. yasm coff\-objfmt\.c 1350 Возможно, здесь забыт оператор 'break;'\.