# Linux\-версия PVS\-Studio устроила себе экскурсию по Disney Недавно вышла в свет Linux\-версия анализатора PVS\-Studio\. С ее помощью был проверен ряд проектов с открытым исходным кодом\. Среди них Chromium, GCC, LLVM \(Clang\) и другие\. И сегодня к этому списку присоединятся проекты, которые были разработаны Walt Disney Animation Studios для сообщества специалистов по созданию виртуальной реальности\. Давайте приступим к рассмотрению найденных предупреждений анализатора\. ![0455_Disney_ru/image1.png](https://import.viva64.com/docx/blog/0455_Disney_ru/image1.png) ## Немного о Disney Компания Walt Disney вот уже много лет радует телевизионную аудиторию разных стран мира восхитительными историями и персонажами, даря ей незабываемые впечатления\. Из года в год Disney выпускает все более увлекательные, зрелищные и сложные по реализации фильмы, и мультфильмы\. Соответственно, увеличивается потребность в разработке различных программ, которые будут способствовать реализации творческих замыслов художников по визуальным эффектам\. Программисты Walt Disney Animation Studios оказывают поддержку специалистам по анимации и визуальным эффектам, создавая технологии, доступные в виде программ на C и C\+\+ с открытым кодом для всех представителей отрасли виртуальной реальности\. К таким программам можно отнести: * Partio \(позволяет работать со стандартными форматами файлов частиц через единый интерфейс, реализованный по тому же принципу, что и унифицированные библиотеки изображений\) * Alembic \(открытый формат обмена, который становится индустриальным стандартом для обмена анимированной компьютерной графикой между пакетами по созданию цифрового контента\) * Universal Scene Description \(эффективная система, способная считывать и передавать данные сцены для обмена между различными графическими приложениями\) * OpenSubdiv \(осуществляет детальный рендеринг поверхностей \(subdivision surface\) на основе уменьшенных моделей\) * Dinamica \(плагин для Autodesk Maya, разработанный на основе физического движка реального времени Bullet Physics Library \) * PTex \(система наложения текстур\) Открытые исходные коды программ от Disney можно скачать на сайте [https://disney\.github\.io/](https://disney.github.io/) \. ## Результаты проверки Рассмотренные проекты от Walt Disney невелики и насчитывают всего несколько десятков тысяч строк кода на C и C\+\+\. Отсюда и такое небольшое количество ошибок по проектам\. ### Проект Partio ![0455_Disney_ru/image2.png](https://import.viva64.com/docx/blog/0455_Disney_ru/image2.png) **Предупреждение PVS\-Studio:** [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression '"R"' is always true\. PDA\.cpp 90 ```cpp ParticlesDataMutable* readPDA(....) { .... while(input->good()) { *input>>word; .... if(word=="V"){ attrs.push_back(simple->addAttribute(....); }else if("R"){ // <= attrs.push_back(simple->addAttribute(....); }else if("I"){ // <= attrs.push_back(simple->addAttribute(....); } index++; } .... } ``` Анализатор выдал сообщение, что условие всегда истина\. Это будет приводить к тому, что действие, которое определено в _else_ ветке, никогда не будет выполнено\. Я считаю, такая ситуация возникла из\-за невнимательности программиста, и условия, которые не будут приводить к такой ошибке, должны выглядеть следующим образом: ```cpp .... if(word=="V"){ attrs.push_back(simple->addAttribute(....); }else if(word=="R"){ // <= attrs.push_back(simple->addAttribute(....); }else if(word=="I"){ // <= attrs.push_back(simple->addAttribute(....); } .... ``` **Предупреждение PVS\-Studio:** [V528](https://pvs-studio.ru/ru/docs/warnings/v528/) It is odd that pointer to 'char' type is compared with the '\\0' value\. Probably meant: \*charArray\[i\] \!\= '\\0'\. MC\.cpp 109 ```cpp int CharArrayLen(char** charArray) { int i = 0; if(charArray != false) { while(charArray[i] != '\0') // <= { i++; } } return i; } ``` Если я правильно понимаю, функция _CharArrayLen _подсчитывает количество символов в строке _charArray_\. Но действительно ли это так? По\-моему, в условии цикла _while_ есть ошибка, связанная с тем, что указатель на тип _char_ сравнивается со значением _'\\0'_\. Высока вероятность, что забыта операция разыменования указателя\. Поэтому условие цикла _while_ должно выглядеть, например, так: ```cpp while ((*charArray)[i] != '\0') ``` Кстати проверка, расположенная чуть выше, тоже весьма странная: ```cpp if(charArray != false) ``` Проверка, конечно, работает, но будет намного лучше заменить её на такую: ```cpp if(charArray != nullptr) ``` В целом, создается впечатление, что функцию разрабатывал стажёр, или она не дописана\. Не понятно, почему бы просто не написать код с использованием функции _strlen\(\)_: ```cpp int CharArrayLen(const char** charArray) { if (charArray == nullptr) return 0; return strlen(*charArray); } ``` **Предупреждение PVS\-Studio:** [V701](https://pvs-studio.ru/ru/docs/warnings/v701/) realloc\(\) possible leak: when realloc\(\) fails in allocating memory, original pointer 'attributeData\[i\]' is lost\. Consider assigning realloc\(\) to a temporary pointer\. ParticleSimple\.cpp 266 ```cpp ParticleIndex ParticlesSimple:: addParticle() { .... for(unsigned int i=0;i void createTopology(....) { .... OpenSubdiv::HbrVertex * destination = mesh->GetVertex( fv[(j+1)%nv] ); OpenSubdiv::HbrHalfedge * opposite = destination->GetEdge(origin); // <= if(origin==NULL || destination==NULL) // <= { printf(....); valid=false; break; } .... } ``` Пожалуй, V595 является самым распространенным предупреждением, выдаваемым анализатором\. PVS\-Studio считает код опасным, если указатель разыменовывается, а потом ниже по коду проверяется\. Если указатель проверяют, то предполагают, что он может быть равен нулю\. Так и происходит в вышеприведенном участке кода\. Для инициализации указателя _opposite_ происходит разыменование указателя _destination,_ а далее идет проверка этих указателей на равенство _NULL_\. И еще парочка предупреждений: * V595 The 'destination' pointer was utilized before it was verified against nullptr\. Check lines: 145, 148\. hbr\_tutorial\_1\.cpp 145 * V595 The 'destination' pointer was utilized before it was verified against nullptr\. Check lines: 215, 218\. hbr\_tutorial\_2\.cpp 215 **Предупреждение PVS\-Studio:** [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'buffer\[0\] \=\= '\\r' && buffer\[0\] \=\= '\\n ' ' is always false\. Probably the '\|\|' operator should be used here\. hdr\_reader\.cpp 84 ```cpp unsigned char *loadHdr(....) { .... char buffer[MAXLINE]; // read header while(true) { if (! fgets(buffer, MAXLINE, fp)) goto error; if (buffer[0] == '\n') break; if (buffer[0] == '\r' && buffer[0] == '\n') break; // <= .... } .... } ``` Программист допустил ошибку в написании условия, которая приводит к тому, что условие всегда равно _false_\. Скорее всего программист хотел сделать так, что если встречаются такие маркеры конца строки, как _\\n_ или _\\r\\n_, то необходимо выйти из цикла _while_\. Поэтому ошибочное условие должно быть записано следующим образом: ```cpp if (buffer[0] == '\r' && buffer[1] == '\n') break; ``` **Предупреждение PVS\-Studio:** [V593](https://pvs-studio.ru/ru/docs/warnings/v593/) Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. main\.cpp 652 ```cpp main(int argc, char ** argv) { .... #if defined(OSD_USES_GLEW) if (GLenum r = glewInit() != GLEW_OK) { // <= printf("Failed to initialize glew. error = %d\n", r); exit(1); } #endif .... } ``` Анализатор обнаружил потенциальную ошибку в выражении _GLenum r \= glewInit\(\) \!\= GLEW\_OK_, которое, скорее всего, работает не так, как задумывал программист\. Создавая такой код, программист, как правило, хочет выполнить действия в следующем порядке: ```cpp (GLenum r = glewInit()) != GLEW_OK ``` Но приоритет оператора '\!\=' выше, чем приоритет оператора присваивания\. Поэтому выражение вычисляется так: ```cpp GLenum r = (glewInit() != GLEW_OK) ``` Поэтому, если функция _glewInit\(\)_ отработает неправильно, на экране будет распечатан неверный код ошибки\. Точнее, всегда будет напечатана единица\. Для исправления ошибки можно использовать скобки или вынести создание объекта за пределы условия, что придаст коду более читабельный вид\. См\. также главу 16 в книге "[Главный вопрос программирования, рефакторинга и всего такого](https://pvs-studio.ru/ru/blog/posts/cpp/0391/)"\. PVS\-Studio обнаружил еще несколько подобных мест: * V593 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. glEvalLimit\.cpp 1419 * V593 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. glStencilViewer\.cpp 1128 * V593 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. farViewer\.cpp 1406 **Предупреждение PVS\-Studio:** [V701](https://pvs-studio.ru/ru/docs/warnings/v701/) realloc\(\) possible leak: when realloc\(\) fails in allocating memory, original pointer 'm\_blocks' is lost\. Consider assigning realloc\(\) to a temporary pointer\. allocator\.h 145 ```cpp template T* HbrAllocator::Allocate() { if (!m_freecount) { .... // Keep track of the newly allocated block if (m_nblocks + 1 >= m_blockCapacity) { m_blockCapacity = m_blockCapacity * 2; if (m_blockCapacity < 1) m_blockCapacity = 1; m_blocks = (T**) realloc(m_blocks, // <= m_blockCapacity * sizeof(T*)); } m_blocks[m_nblocks] = block; // <= .... } .... } ``` И снова опасное использование функции _realloc_\. А почему оно опасное \- описано выше в разделе 'Проект Partio'\. ### Проект Dynamica ![0455_Disney_ru/image6.png](https://import.viva64.com/docx/blog/0455_Disney_ru/image6.png) **Предупреждение PVS\-Studio:** [V512](https://pvs-studio.ru/ru/docs/warnings/v512/) A call of the 'memset' function will lead to overflow of the buffer 'header\.padding'\. pdbIO\.cpp 249 ```cpp struct pdb_header_t { int magic; unsigned short swap; float version; float time; unsigned int data_size; unsigned int num_data; char padding[32]; //pdb_channel_t **data; int data; }; bool pdb_io_t::write(std::ostream &out) { pdb_header_t header; .... header.magic = PDB_MAGIC; header.swap = 0; header.version = 1.0; header.time = m_time; header.data_size = m_num_particles; header.num_data = m_attributes.size(); memset(header.padding, 0, 32 * sizeof(char) + sizeof(int)); .... } ``` Анализатор обнаружил потенциально возможную ошибку, связанную с заполнением буфера памяти _header\.padding_\. Через _memset _программист обнуляет 36 байтов в буфере _header\.padding_, состоящий всего из 32 байт_\._ На первый взгляд такое использование ошибочно, но, на самом деле, программист оказался хитрецом и вместе с _header\.padding_ обнуляет переменную _data\._ Ведь поля _padding_ и _data_ структуры _pdb\_header\_t_ расположены последовательно, а значит последовательно расположены и в памяти\. Да\! Ошибки нет в данной ситуации, но из\-за такой хитрости в этом месте потенциально может появиться ошибка\. Например, если другой программист изменит структуру _pdb\_header\_t_, добавив между полями _padding_ и _data_ свои поля, и не заметит хитрости своего коллеги\. Поэтому лучше обнулять каждую переменную по отдельности\. ### Проект Ptex ![0455_Disney_ru/image7.png](https://import.viva64.com/docx/blog/0455_Disney_ru/image7.png) **Предупреждение PVS\-Studio:** [V612](https://pvs-studio.ru/ru/docs/warnings/v612/) An unconditional 'return' within a loop\. PtexHashMap\.h 292 ```cpp Entry* lockEntriesAndGrowIfNeeded(size_t& newMemUsed) { while (_size*2 >= _numEntries) { Entry* entries = lockEntries(); if (_size*2 >= _numEntries) { entries = grow(entries, newMemUsed); } return entries; } return lockEntries(); } ``` В вышеприведенной функции присутствует подозрительный цикл _while_, в котором при первом же проходе возвращается указатель на _entries_\. Не кажется ли вам, что здесь что\-то запутанное? Этот участок кода требует более детального рассмотрения\. ## Заключение Статический анализ кода при написании качественного ПО играет очень важную роль, так как используя статический анализ на регулярной основе, вы сокращаете трудозатраты на устранение глупых или тяжело обнаруживаемых ошибок и сможете потратить больше времени на что\-то полезное\. Если вы еще не проверяли свой проект на наличие ошибок и не пускались в увлекательные поиски багов, то советую Вам скачать [PVS\-Studio для Linux](https://pvs-studio.ru/ru/pvs-studio/download/) и обязательно это сделать\.