# Команда PVS\-Studio расширяет кругозор, выполняя разработку на заказ

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

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

Нам хотелось побывать в шкуре своих клиентов для того, чтобы понять, как наш инструмент используется в долгосрочных проектах\. Ведь проверки проектов, которые мы делаем регулярно и, про которые пишем много [статей](https://pvs-studio.ru/ru/blog/inspections/), это как раз тот стиль использования анализатора, против которого мы активно выступаем\. Неправильно запустить разово анализатор на проекте, исправить несколько ошибок и повторить это через год\. При написании кода анализатор надо использовать регулярно, каждый день\.

Ну да ладно, к чему это все? Наши теоретические желания попробовать себя в других проектах совпали с практическими предложениями, которые постепенно стали к нам поступать\. В прошлом году мы решили выделить у нас в компании команду, которая бы занималась – о ужас\! – разработкой на заказ\. То есть участвовала в сторонних проектах в качестве программистов\. Причем нам было интересно участвовать в долгосрочных и довольно крупных проектах, т\.е\. не менее 2\-3 разработчиков и не менее 6 месяцев разработки\. У нас было две цели: 

* попробовать альтернативный тип бизнеса \(заказную разработку помимо продуктовой разработки\);
* самим посмотреть на использование PVS\-Studio в долгосрочных проектах\.

И первая, и вторая задача оказались удачными\. Но эта статья не про бизнес по заказной разработке, а про наш опыт\. Имеется в виду не организационный опыт\. Про это и так много статей\. Про опыт работы с кодом чужих проектов\. Про это мы и хотим рассказать\. 

## Замеченные интересные моменты

Сторонние проекты многому учат нас\. Думаю, этой тематике мы посвятим далеко не одну статью\. А сейчас мы начнём с некоторых интересных наблюдений\. Мы заметили 3 яркие особенности кода, которые проявили себя в больших и старых проектах\. Уверены, со временем будут публикации и про другие наблюдения\.

У нас есть достаточно много [статей](https://pvs-studio.ru/ru/blog/posts/cpp/0188/), посвященных смене разрядности платформы с 32\-бит на 64\-бита\. С портированием связано множество ошибок, которые начинают проявлять себя в 64\-битных программах\. Мы называем их [64\-битные ошибки](https://pvs-studio.ru/ru/blog/terms/0002/)\.

Но помимо перехода на 64\-битные системы, произошли и другие изменения в коде, которые не так заметны\. Произошло это в связи с развитием компиляторов, библиотек и взрослением самих проектов\. Последствия этих изменений хорошо заметны в коде, имеющем длинную историю развития\. Предлагаем их обсудить\. Думаю, это будет интересно и полезно\. Возможно, кто\-то захочет провести обзор своего старого кода, чтобы выявить аналогичные проблемы\. 

Рассмотренные паттерны ошибок были замечены нами, благодаря анализатору PVS\-Studio  \(см\. [возможности анализаторов](https://pvs-studio.ru/ru/blog/posts/0258/)\)\.  Многие из этих ошибок скрыты\. Код почти работает благодаря удачному стечению обстоятельств\. Однако, каждая такая ошибка \- маленькая бомба замедленного действия, которая может сработать в любой момент\.

**Примечание\.** Чтобы избежать ограничений NDA, мы изменили имена и отредактировали код\. Собственно, это совсем не тот код, который был в программе\. Однако, "всё основано на реальных событиях"\.

### Изменение в поведении оператора new

Давным\-давно оператор 'new' возвращал NULL в случае ошибки выделения памяти\. Затем компиляторы начали поддерживать современный стандарт языка Си\+\+, и бросать в таких ситуациях исключения std::bad\_alloc\. Можно заставить оператор 'new' возвращать NULL, но сейчас речь не об этом\.

Видимо, эти изменения были замечены сообществом программистов весьма поверхностно\. Взяли себе на заметку этот факт и начали писать код с учётом нового поведения\. Есть, конечно, программисты, которые до сих пор не в курсе, что оператор 'new' кидает исключение\. Но мы говорим о нормальных, адекватных программистах\. Личности, которые ничего не хотят знать и продолжают писать в стиле, как делали это 20 лет назад, нам не интересны\. 

Тем не менее, даже те, кто знает, что оператор 'new' изменил своё поведение, не провели обзор существующего кода\. Кто\-то просто не сообразил это сделать\. Кто\-то хотел, но ему было некогда, а потом он забыл про это\. В результате, в огромном количестве программ продолжают жить некорректные обработчики ситуаций, когда невозможно выделить память\.

Некоторые фрагменты кода безобидны:

```cpp
int *x = new int[N];
if (!x) throw MyMemoryException(); // 1
int *y = new int[M];
if (!y) return false; // 2
```

В первом случае вместо MyMemoryException будет сгенерировано исключение std::bad\_alloc\. Скорее всего, эти исключения обрабатываются одинаковым образом\. Проблем нет\.

Во втором случае проверка не сработает\.  Функция не вернёт значение 'false'\. Вместо этого возникнет исключение, которое как\-то будет потом обработано\. Это ошибка\. Поведение программы изменилось\. Но на практике это почти никогда не приводит к проблемам\. Просто программа немного по\-разному будет реагировать на нехватку памяти\.

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

Вот пара примеров, когда при нехватке памяти должны быть выполнены определённые действия:

```cpp
image->SetBuffer(new unsigned char[size]);
if (!image->GetBuffer())
{
  CloseImage(image);
  return NULL;
}

FormatAttribute *pAttrib = new FormatAttribute(sName, value, false);
if (pAttrib )
{
    m_attributes.Insert(pAttrib);
}
else
{
  TDocument* pDoc = m_state.GetDocument();
  if (pDoc)
     pDoc->SetErrorState(OUT_OF_MEMORY_ERROR, true);
}
```

Такой код намного более опасен\. Например, пользователь может потерять содержимое своего документа в случае нехватки памяти, хотя вполне можно было дать возможность сохранить документ в файл\.

**Рекомендация\.** Найдите в своей программе все операторы 'new'\. Проверьте, не собирается ли программа предпринять какие\-то действия, если указатель будет нулевым\. Перепишите такие места\.

Анализатор PVS\-Studio помогает обнаружить бессмысленные проверки с помощью диагностики [V668](https://pvs-studio.ru/ru/docs/warnings/v668/)\.

### Замена char \* на std::string

С переходом от Си к Си\+\+ и с ростом популярности стандартной библиотеки, в программах стали широко использовать классы строк, такие как std::string\. Это понятно и объяснимо\. Проще и безопасней работать с полноценной строкой, а не с указателем "char \*"\.

Класс строки стали использовать не только в новом коде, но и изменять некоторые старые фрагменты\. Как раз с этим и могут возникать неприятности\. Достаточно ослабить внимание, и код становится опасным или вовсе некорректным\.

Но начнём не со страшного, а скорее забавного\. Иногда попадаются вот такие неэффективные циклы: 

```cpp
for (i = 0; i < strlen(str.c_str()); ++i)
```

Явно, когда\-то переменная 'str' была обыкновенным указателем\. Плохо вызывать функцию strlen\(\) при каждой итерации цикла\. Это крайне неэффективно на длинных строках\. Однако, после превращения указателя в std::string, код стал выглядеть ещё более глупо\.

Сразу видно, что замена типов происходила бездумно\. Это может приводить к подобному неэффективному коду или вообще к ошибкам\. Поговорим про ошибки:

```cpp
wstring databaseName;
....
wprintf("Error: cannot open database %s", databaseName); // 1

CString s;
....
delete [] s; // 2
```

В первом случае, указатель 'wchar\_t \*' превратился в 'wstring'\. Беда возникнет, если не удастся открыть базу, и нужно будет распечатать сообщение\. Код компилируется, но на экран будет распечатана абракадабра\. Или программа вообще упадёт\. Причина \- забыли добавить вызов функции c\_str\(\)\. Корректный вариант:

```cpp
wprintf("Error: cannot open database %s", databaseName.c_str());
```

Второй случай вообще эпичен\. Как это не удивительно, но такой код успешно компилируется\. Используется весьма популярный строковый класс CString\. Класс CString неявно может преобразовываться к указателю\. Именно это здесь и происходит\. Результат \- двойное удаление буфера\.

**Рекомендация\.** Если вы меняете указатели на класс строки, делайте это аккуратно\. Не используйте массовую замену без просмотра каждого случая\. То, что код компилируется, вовсе не означает, что он работает\. Лучше оставить в покое код, использующий указатели, если нет явной необходимости его править\. Пусть лучше код корректно работает с указателями, чем некорректно с классами\.

Анализатор PVS\-Studio помогает обнаружить некоторые из ошибок, возникшие из\-за замены указателей на классы\. В этом могут помочь диагностики [V510](https://pvs-studio.ru/ru/docs/warnings/v510/), [V515](https://pvs-studio.ru/ru/docs/warnings/v515/) и т\.д\. Но полностью надеяться на анализаторы не стоит\. Может попасться крайне творческий код, ошибку в котором с трудом сможет найти не только анализатор, но и опытный программист\.

### Замена char на wchar\_t

Проект развивается, появляется желание сделать интерфейс приложения многоязычным\. И вот в какой\-то момент происходит массовая замена char на wchar\_t\. Поправлены ошибки, выданные компилятором\. "Готова" unicode\-версия приложения\.

На практике, после такой замены приложение превращается в решето\. Ошибки, которые возникнут после такой замены, могут проявлять себя десятилетиями и с трудом воспроизводиться\. 

Как\-же такое может быть? Очень просто\. Многие фрагменты кода не готовы, что размер символа изменится\. Код компилируется без ошибок и без предупреждений\. Но работает только на "50%"\. Сейчас вы поймете, что мы имеем в виду\.

**Примечание\.** Напомню, что мы не запугиваем плохим кодом, полученным от студентов\. Мы рассказываем о суровых реалиях программистского бытия\. В больших и старых проектах неизбежно появляются такие ошибки\. И их сотни\. Серьёзно\. Сотни\.

Примеры:

```cpp
wchar_t tmpBuffer[500];
memset(tmpBuffer, 0, 500); // 1

TCHAR *message = new TCHAR[MAX_MSG_LENGTH];
memset(charArray, 0, MAX_MSG_LENGTH*sizeof(char)); // 2

LPCTSTR sSource = ...;
LPTSTR sDestination = (LPTSTR) malloc (_tcslen(sSource) + 12); // 3

wchar_t *name = ...;
fprintf(fp, "%i : %s", i, name); // 4
```

В первом случае программист вообще не задумался, что размер символа со временем изменится\. Поэтому очистил только половину буфера\. Вот откуда мои слова про 50%\. 

Во втором случае программист догадывался, что размер символа изменится\. Однако, подсказка "\* sizeof\(char\)" никак не помогла при массовой замене типов\. Надо было делать не так\. Правильный вариант:

```cpp
memset(charArray, 0, MAX_MSG_LENGTH * sizeof(charArray[0]));
```

Третий пример\. С типами всё хорошо\. Для подсчёта длины строки используется функция \_tcslen\(\)\. На первый взгляд всё замечательно\. Однако, когда символы стали иметь тип 'wchar\_t', программа опять стала работать на 50%\.

Выделяется в 2 раза меньше памяти, чем необходимо\. По факту, программа будет успешно работать до тех пор, пока длина сообщения не будет превышать половину от максимально возможной длины\. Неприятная ошибка которая надолго задержалась в коде\.

Четвёртый пример\. Поправили функции printf\(\), sprintf\(\) и так далее, но забыли проверить frintf\(\)\. В результате, в файл записывается мусор\. Надо было сделать как\-то так:

```cpp
fwprintf(fp, L"%i : %s", i, name);
```

Или так, если это обыкновенный ASCII\-файл:

```cpp
fprintf(fp, "%i : %S", i, name);
```

Примечание\. Сейчас подумал, что говорил про 50%, с точки зрения Windows\. В Linux тип wchar\_t занимает не 2, а 4 байта\. Так что в Linux мире программа будет работать вообще только на 25% :\)\. 

**Рекомендация\.** Если вы уже массово заменили char на wchar\_t, не знаем, что может помочь\. Этим можно было внести массу интересных ошибок\. Внимательно просмотреть все места, где используется wcahr\_t, не реально\. Отчасти вам помогут статические анализаторы кода\. Часть ошибок они выявят\. Показанные выше ошибки найдет анализатор PVS\-Studio\. Последствия замены char на wchar\_t весьма разнообразны и выявляются различными диагностиками\. Перечислять их не имеет смысла\. В качестве примера, можно назвать [V512](https://pvs-studio.ru/ru/docs/warnings/v512/), [V576](https://pvs-studio.ru/ru/docs/warnings/v576/), [V635](https://pvs-studio.ru/ru/docs/warnings/v635/) и т\.д\.

Если вы ещё не делали замену, но планируете, то подойдите к ней со всей серьезностью\. Заменить типы автоматически и поправить ошибки компиляции займет, например, 2 дня\. Сделать те же замены вручную, с просмотром кода, займет на порядок больше времени\. Например, вы потратите на это 2 недели\. Но это лучше, чем потом втечение 2 лет вылавливать все новые ошибки:

* неправильное форматирование строк;
* выход за границы выделенных буферов памяти;
* работа с буферами, очищенными на половину;
* работа с буферами, скопированными на половину;
* и так далее\.

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

Мы остались довольны своим опытом участия в сторонних заказных разработках, так как это позволило нам посмотреть на мир другими глазами\. Мы продолжим свое участие в сторонних проектах в качестве разработчиков, так что, если кому интересно пообщаться с нами на эту тему, пишите\. Если боитесь, что потом про вас будет разоблачительная статья \(про найденные ошибки\), то все\-равно [пишите](https://pvs-studio.ru/ru/about-feedback/) – да поможет нам всем NDA\!