# Как программисты с PVS\-Studio ошибки в проектах искали

Недавно сайт Pinguem\.ru совместно с командой PVS\-Studio устраивали конкурс, в котором программистам было необходимо в течение месяца использовать статический анализатор PVS\-Studio для нахождения и исправления ошибок в коде open\-source проектов\. Благодаря их стараниям, программы в мире стали чуточку безопаснее и надежнее\. В статье мы рассмотрим парочку наиболее интересных ошибок, которые были найдены при помощи PVS\-Studio\.

![0544_Results_of_Pinguem_contest_ru/image1.png](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest_ru/image1.png)

## И как прошел конкурс?

Конкурс проходил с 23 октября по 27 ноября 2017 года в два этапа для русскоязычной аудитории\. На первом этапе конкурсантам было необходимо отправить как можно больше Pull Request'ов авторам проектов\. Второй этап был несколько сложнее: нужно было найти ошибку и описать последовательность действий, при которых она бы себя проявила в работе приложения\. Лучше всех с заданиями справился [Николай Шалакин](https://github.com/AskePit) и стал победителем конкурса\. Поздравляем его\! 

За время проведения конкурса было отправлено немало хороших Pull Request'ов, желающие могут ознакомиться с ними по этой [ссылке](https://github.com/search?o=desc&q=pinguem+pvs&s=comments&type=Issues&utf8=%E2%9C%93)\. Мы же предлагаем рассмотреть наиболее интересные ошибки, найденные конкурсантами на втором этапе\.

### QtCreator

Многие ли из Вас используют QtCreator для программирования на Python? Как и многие IDE, он тоже подсвечивает некоторые встроенные функции и объекты\. Возьмем QtCreator 4\.4\.1 и напишем несколько зарезервированных слов:

![0544_Results_of_Pinguem_contest_ru/image2.gif](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest_ru/image2.gif)

Что же такое? Почему встроенные функции _oct_ и _chr_ не подсвечиваются? Взглянем на код:

```cpp
// List of python built-in functions and objects
static const QSet<QString> builtins = {
"range", "xrange", "int", "float", "long", "hex", "oct" "chr", "ord",
"len", "abs", "None", "True", "False"
};
```

Функции объявлены, они должны подсвечиваться\. И здесь помогает PVS\-Studio:

[V653](https://pvs-studio.ru/ru/docs/warnings/v653/) A suspicious string consisting of two parts is used for initialization\. It is possible that a comma is missing\. Consider inspecting this literal: "oct" "chr"\. pythonscanner\.cpp 205

Действительно, между литералами "oct" и "chr" забыли поставить запятую, поэтому два литерала слились в один "octchr", и именно он будет подсвечиваться средой разработки:

![0544_Results_of_Pinguem_contest_ru/image3.gif](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest_ru/image3.gif)

[Здесь](https://github.com/qt-creator/qt-creator/pull/21) можно ознакомиться с Pull Request'ом на исправление\.

### ConEmu

Вы работаете над проектом ConEmu и в отладочной версии решили проверить работу некоторых настроек \(нажмите на анимацию для увеличения\):

![](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest_ru/image4.mp4)

Давайте взглянем на код, почему вылетает предупреждение "ListBox was not processed":

```cpp
INT_PTR CSetPgViews::OnComboBox(HWND hDlg, WORD nCtrlId, WORD code)
{
  switch (code)
  {
  ....
  case CBN_SELCHANGE:
    {
      ....
      UINT val;
      INT_PTR nSel = SendDlgItemMessage(hDlg, 
                                        nCtrlId, 
                                        CB_GETCURSEL,
                                        0,
                                        0);
      switch (nCtrlId)
      {
        ....
        case tThumbMaxZoom:
          gpSet->ThSet.nMaxZoom = max(100,((nSel+1)*100));
        default:
          _ASSERTE(FALSE && "ListBox was not processed");
      }
    }
  }
}
```

Из\-за пропущенного _break_ управление перейдет к ветке default после того, как отработают выражения в ветке _tThumbMaxZoom_\. Об этом предупреждает PVS\-Studio:

[V796](https://pvs-studio.ru/ru/docs/warnings/v796/) It is possible that 'break' statement is missing in switch statement\. setpgviews\.cpp 183

[Здесь](https://github.com/Maximus5/ConEmu/pull/1328) можно ознакомиться с Pull Request'ом на исправление\.

### UniversalPauseButton

Довольно интересный проект, особенно полезный для геймеров\. При нажатии на кнопку _Pause_ программа приостанавливает работу окна, находящегося на переднем плане:

<https://www.youtube.com/watch?v=bItgybhViQM>

Можно переназначить кнопку приостановки/возобновления на другую при помощи файла _settings\.txt_:

![0544_Results_of_Pinguem_contest_ru/image6.png](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest_ru/image6.png)

Если ввести код ключа не менее, чем 20 символов, и не более, чем 30 символов, это приведет к порче стека \(нажмите на анимацию для увеличения\):

![](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest_ru/image7.mp4)

Разберемся, почему это происходит\. Нас интересует функция _LoadPauseKeyFromSettingsFile_:

```cpp
int LoadPauseKeyFromSettingsFile(_In_ wchar_t* Filename)
{
  HANDLE FileHandle = CreateFile(Filename, 
                                 GENERIC_READ,
                                 FILE_SHARE_READ,
                                 NULL,
                                 OPEN_EXISTING,
                                 FILE_ATTRIBUTE_NORMAL,
                                 NULL);

  if (FileHandle == INVALID_HANDLE_VALUE)
  {
    goto Default;
  }
  
  char  KeyLine[32] = { 0 };
  char  Buffer[2]   = { 0 };
  DWORD ByteRead    = 0;

  do
  {
    if (!ReadFile(FileHandle, Buffer, 1, &ByteRead, NULL))
    {
      goto Default;
    }

    if (Buffer[0] == '\r' || Buffer[0] == '\n')
    {
      break;
    }

    size_t Length = strlen(KeyLine);
    if (Length > 30)                                            // <=
    {
      goto Default;
    }

    KeyLine[Length] = Buffer[0];    
    memset(Buffer, 0, sizeof(Buffer));
  } while (ByteRead == 1);

  if (!StringStartsWith_AI(KeyLine, "KEY="))
  {
    goto Default;
  }

  char KeyNumberAsString[16] = { 0 };                           // <=

  for (DWORD Counter = 4; Counter < strlen(KeyLine); Counter++) // <=
  {
    KeyNumberAsString[Counter - 4] = KeyLine[Counter];
  }
  ....

  Default:
  if (FileHandle != INVALID_HANDLE_VALUE && FileHandle != NULL)
  {
    CloseHandle(FileHandle);    
  }
  return(0x13);
}
```

В цикле считывается побайтово первая строка\. Если она превышает длину в 30 символов, то выполнение переходит по метке _Default_,_ _при этом освобождается ресурс и возвращается символ с кодом 0x13\. Если чтение происходит успешно, и первая строка начинается с "KEY\=", то происходит копирование подстроки после символа "\=" в 16\-байтовый буфер _KeyNumberAsString_\. При вводе ключа от 20 до 30 символов произойдет переполнение буфера\. Об этом предупреждает PVS\-Studio:

[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'Counter \- 4' index could reach 26\. main\.cpp 146

[Здесь](https://github.com/ryanries/UniversalPauseButton/pull/11) можно ознакомиться с Pull Request'ом на исправление\.

### Explorer\+\+

В этом проекте был найден баг, связанный с сортировкой избранных вкладок \(нажмите на анимацию для увеличения\):

![](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest_ru/image9.mp4)

Посмотрим на код сортировки:

```cpp
int CALLBACK SortByName(const NBookmarkHelper::variantBookmark_t
                          BookmarkItem1,
                        const NBookmarkHelper::variantBookmark_t
                          BookmarkItem2)
{
  if (   BookmarkItem1.type() == typeid(CBookmarkFolder)
      && BookmarkItem2.type() == typeid(CBookmarkFolder))
  {
    const CBookmarkFolder &BookmarkFolder1 =
      boost::get<CBookmarkFolder>(BookmarkItem1);
    const CBookmarkFolder &BookmarkFolder2 =
      boost::get<CBookmarkFolder>(BookmarkItem2);

    return BookmarkFolder1.GetName()
           .compare(BookmarkFolder2.GetName());
  }
  else
  {
    const CBookmark &Bookmark1 = 
      boost::get<CBookmark>(BookmarkItem1);
    const CBookmark &Bookmark2 =
      boost::get<CBookmark>(BookmarkItem1);

    return Bookmark1.GetName().compare(Bookmark2.GetName());
  }
}
```

В ветке _else_ ошиблись и дважды использовали _BookmarkItem1_, вместо _BookmarkItem2_\. Об этой ошибке и предупреждает PVS\-Studio:

* [V537](https://pvs-studio.ru/ru/docs/warnings/v537/) Consider reviewing the correctness of 'BookmarkItem1' item's usage\. bookmarkhelper\.cpp 535
* и еще 5 дополнительных предупреждений\.

[Здесь](https://github.com/derceg/explorerplusplus/pull/85) можно ознакомиться с Pull Request'ом на исправление\.

## Заключение

Команда PVS\-Studio выражает огромную благодарность всем программистам, принявшим участие в конкурсе\. Вы проделали огромную работу в устранении багов в open\-source проектах, сделав их надежнее, безопаснее и лучше\. В будущем мы, возможно, проведем подобный конкурс и для англоязычной аудитории\.

Мы также предлагаем всем остальным [скачать](https://pvs-studio.ru/ru/pvs-studio/download/) и попробовать анализатор PVS\-Studio, он прост в обращении и может быть Вам полезен\.