Разбор С++ квиза от Сергея Кушниренко

• Задача N1. Кручу - верчу - перевернуть хочу
• Задача N2. Хеш-суммы
• Задача N3. Потоки
• Задача N4. Мьютекс
• Задача N5. Опасная функция с переменным количеством аргументов
• Задача N6. unique_lock
• Задача N7. memcmp
• Задача N8. operator =
• Задача N9. Строка в DLL
• Задача N10. serialize
• Задача N11. sayHello
• Задача N12. Модуль числа
• Задача N13. Где деньги, Лебовски?
• Задача N14. cin
• Задача N15. display_string
• Задача N16. Бар
• Задача N17. Бард
• Задача N18. Итераторы
• Всем спасибо
• Дополнительные ссылки

Команда PVS-Studio вместе с Сергеем Кушниренко подготовила квиз на основе его публикаций. Вам предстоит попробовать найти ошибки в C++ коде и проверить свою внимательность и знание языка. В этой статье сделан подробный разбор всех предлагаемых вопросов.

Если вы ещё не проходили квиз, а открыли эту статью, то предлагаем не спешить её читать. Интерес в том, чтобы самим попробовать понять, где находится ошибка в коде.

Пройдите квиз и возвращайтесь.

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

Квиз появился благодаря публикациям Сергея Кушниренко на сайте Habr. Команда PVS-Studio предложила оформить рассматриваемые в статьях фрагменты кода в виде развлекательного задачника.

Спасибо Сергею, что он согласился поучаствовать в этом проекте.

Задача N1. Кручу - верчу - перевернуть хочу

Этот код использовался в движке Unity 2014 для управления поворотом объектов с помощью гизмо (такой элемент управления обычно в форме трех окружностей, который позволяет крутить объект на сцене). В данном случае функция setDeltaPitch применяется для изменения угла наклона объекта относительно его вертикальной оси (pitch). При углах, близких к 0 (зависело от настроек редактора), функция просто переворачивала объект вверх ногами, что очень бесило дизайнеров уровней.

void  UObject::setDeltaPitch(const UMatrix &gizmo) {
  ....
  if (_fpzero(amount, eps))
    return
  rotateAccum.setAnglesXYZ(axis);
  ....
}

Варианты ответа:

• На самом деле здесь всё правильно. Это пример для разминки.
• У функции _fpzero местами перепутаны фактические аргументы.
• Класс UMatrix оперирует типами float, из-за чего не хватает точности. Следовало использовать UMatrixD, основанную на типах double.
• Оформление кода не соответствует логике его работы. [ПРАВИЛЬНО]

Ответ:

После return отсутствует точка с запятой, что при определённых условиях приводило к вызову функции setAnglesXYZ в управляемом объекте и переворачивало его на произвольный угол.

Примечание команды PVS-Studio

Кажется, что ошибки с забытой точкой с запятой весьма распространены. Они часто упоминаются в разных статьях. Но на самом деле они встречаются очень редко. Более чем за 10 лет проверки открытых проектов мы нашли всего 3 таких ошибки. Сказывается, что все знают про эти ошибки и внимательны к ним. Плюс анализаторы и компиляторы, видимо, тоже хорошо помогают обнаруживать их.

Задача N2. Хеш-суммы

Тут повеселился компилятор. Эта функция использовалась для вычисления хеш-суммы при проверке целостности файлов. При создании контента вычислялся хеш файлов и сохранялся вместе с файлами. Позже при загрузке того же файла плеер Unity снова вычислял хеш файла и сравнивал его с сохранённым, чтобы убедиться, что файл не был изменён или повреждён. Шифрование использовалось на финальном этапе упаковки. По замыслу автора этого кода, ключ не должен был утечь за пределы этой функции, но что-то пошло не так. Наличие части утёкших исходников Unity и этого бага в движке помогло SKIDROW в 2017 взломать квест Syberia 3 (Сибирь 3) и еще несколько больших игр на этом движке, которые использовали нативные средства шифрования контента. Там, конечно, ещё был Denuvo, но его SKIDROW научились обходить ещё до этого.

void UContentPackage::createPackage(dsaFile *m, const void *v, int size) {
  unsigned char        secretDsaKey[3096], L;
  const unsigned char *p = v;
  int                  i, j, t;
  ....
  UContent::generateDsaKey(secretDsaKey, sizeof(salt));
  ....
  // тут был какой-то код для шифрования и подписи файла
  ....
  memset (secretDsaKey, 0, sizeof(secretDsaKey));
}

Варианты ответа:

• Функция memset ненастоящая. Выше сознательно внесена уязвимость, сводящаяся к переопределению функции memset.
• Компилятор удаляет вызов функции memset, так как с его точки зрения она не нужна. [ПРАВИЛЬНО]
• Компилятор удаляет вызов функции memset из-за редкого бага. Баг был исправлен в следующей версии компилятора, но именно когда баг обнаружился, использовалась та самая неудачная версия.
• Ошибка в функции generateDsaKey. Она переполняет буфер, и часть данных оказывается вообще не пойми где.

Ответ

Функция memset не вызывалась из-за агрессивной оптимизации компилятора. Действия с secretDsaKey не проводились после memset, поэтому компилятор её просто выкидывал. Всё содержимое ключа оставалось на стеке.

Примечание команды PVS-Studio

Здесь ситуация по сравнению с предыдущим паттерном ошибки полностью противоположная. Ошибка с исчезновением memset кажется чем-то экзотическим и редким. Но это очень распространённая потенциальная уязвимость. На момент написания этой заметки наша коллекция уже насчитывает 330 таких ошибок в разнообразнейших открытых проектах и продолжает пополняться. Хотя, казалось бы, на эту тему тоже уже много написано, например:

• CWE-14;
• Zero and forget -- caveats of zeroing memory in C;
• Безопасная очистка приватных данных.

Также приватные данные могут быть отправлены наружу, например по сети. Как? Рассматриваем в заметке "Перезаписывать память - зачем?".

Задача N3. Потоки

Здесь могут быть проблемы при работе из двух и более потоков с любой из переменных a/b. Эта ошибка присутствовала в движке CryTek при синхронизации состояний автомобилей по сети, из-за чего наблюдались рывки и телепорты во время движения на авто в мультиплеере FarCry 1. Чем больше игроков было на карте, тем больше была вероятность телепорта у последнего игрока. При 16 игроках на карте последнего стабильно телепортировало при использовании авто.

struct X {
  int a : 2
  int b : 2
} x;

Thread 1:
void foo() { x.a = 1 }

Thread 2:
void boo() { x.b = 1 }

Варианты ответа:

• Записи в поля a и b не являются атомарными. Они могут выполняться частично и прерываться другими операциями. [ПРАВИЛЬНО]
• Из-за бага в компиляторе игнорировался размер первого поля. Это приводило к тому, что при записи в переменную a также перезаписывалась и b.
• Забыли, что один бит тратится под знак, из-за чего диапазон значений двухбитовых переменных оказался недостаточен: [-2..1]. Нужно было делать поля из 3 бит.

Ответ

Нарушение атомарности операции записи. Поля a и b не являются атомарными, что означает, что они могут выполняться частично и прерываться другими операциями. В данном коде присутствует общий доступ к комплексной переменной AB, которая состоит из двух двубитовых частей. Но компилятор не может выполнить такое присвоение атомарно, захватывая сразу байт ab и битовыми операциями выставляя нужное значение. Это может привести к гонкам данных и неопределённому поведению в многопоточной среде.

foo():                                # @foo()
        mov     AL, BYTE PTR [RIP + x] ; разорваное присвоение 1
        and     AL, -4                 ; разорваное присвоение 2
        or      AL, 1                  ; разорваное присвоение 3
        mov     BYTE PTR [RIP + x], AL ; закончили
        ret

boo():                                # @boo()
        mov     AL, BYTE PTR [RIP + x]
        and     AL, -13
        or      AL, 4
        mov     BYTE PTR [RIP + x], AL
        ret

Задача N4. Мьютекс

Этот код содержит гонку данных даже при наличии "вроде бы живого" мьютекса. Ошибка была замечена в прошивке Nintendo Switch 4.5.1 и выше. Наткнулись на неё совершенно случайно, когда пытались ускорить создание атласов UI текстур на старте игры. Если пробовали загрузить в атлас больше 100 текстур, то ломали её. А если меньше, то атлас собирался нормально. Такие мьютексы-зомби на Nintendo Switch до сих пор не пофикшены. А ещё на Nintendo Switch можно было сделать не больше 256 мьютексов на приложение, вот такая вот весёлая система.

const size_t maxThreads = 10;
 
void fill_texture_mt(int thread_id, std::mutex *pm) {
  std::lock_guard<std::mutex> lk(*pm);
  
  // Access data protected by the lock.
}
 
void prepare_texture() {
  std::thread threads[maxThreads];
  std::mutex m;
 
  for (size_t i = 0; i < maxThreads; ++i) {
    threads[i] = std::thread(fill_texture_mt, i, &m);
  }
}

Варианты ответа:

• Выбран неправильный стандартный класс для реализации блокировок.
• Отсутствует ожидание завершения работы потоков. Из-за этого мьютекс удаляется сразу по завершении функции prepare_texture, и блокировки потоков не происходит. [ПРАВИЛЬНО]
• В приведённом коде всё правильно. Другое дело, что это неэффективный способ распараллеливания. Но это отдельная тема.
• Переменная m должна быть объявлена со спецификатором volatile.

Ответ

Мьютекс удаляется сразу по завершении функции prepare_texture. Операционная система никак не реагирует на неактивный мьютекс, потому что, как объект ядра, он продолжает еще некоторое время жить. Адрес является валидным и содержит корректные данные, но не предоставляет реальной блокировки потоков.

Примечание. Судя по документации, при выходе из функции позовутся деструкторы активных std::thread, что должно приводить к аварийному завершению работы (вызову terminate). Однако, по словам автора, на этом проекте такого не происходило. Непонятный момент. Впрочем, это не так важно, ведь код в любом случае некорректен.

Задача N5. Опасная функция с переменным количеством аргументов

Функции могут быть определены так, чтобы принимать на месте вызова больше аргументов, чем указано в объявлении. Такие функции называются вариативными (variadic). C++ предоставляет два механизма, с помощью которых можно определить вариативную функцию: шаблон с переменным числом параметров и использование многоточия в стиле C в качестве окончательного объявления параметра. Очень неприятное поведение было встречено в популярной библиотеке для работы со звуком FMOD Engine. Код приводится в том виде, какой он был в исходниках: похоже, ребята хотели сэкономить на шаблонах. Поиграть с кодом на сайте OnlineGDB.

int var_add(int first, int second, ...) {
  int r = first + second; 
  va_list va;
  va_start(va, second);
  while (int v = va_arg(va, int)) {
    r += v;
  }
  va_end(va);
  return r;
}

Варианты ответа:

• Объявление переменной va должно быть в самом начале функции до переменной r.
• Последним параметром функции должен быть 0. Иначе возникнет неопределённое поведение.
• Передача любого типа кроме int приводит к неопределённому поведению.
• Одновременно верны 2 и 3. [ПРАВИЛЬНО]
• Приведённый код корректен. Ошибка является наведённой извне (например, из-за игр с макросами).

Ответ

Это вариативная функции в стиле C для сложения ряда целых чисел. Аргументы будут считываться до тех пор, пока не будет найдено значение 0. Вызов этой функции без передачи значения 0 в качестве завершающего аргумента (после первых двух аргументов) приводит к неопределённому поведению. Более того, передача любого типа кроме int также приводит к неопределённому поведению.

Задача N6. unique_lock

А вот тут мы опять ничего не залочили, хотя на первый взгляд залочили. Этот код был обнаружен в движке Metro: Exodus в некоторых местах при работе с ресурсами, что приводило к странным крашам. Нашли благодаря баг-репортам и одному умному французу.

static std::mutex m;
static int shared_resource = 0;
 
void increment_by_42() {
  std::unique_lock<std::mutex>(m);
  shared_resource += 42;
}

Варианты ответа:

• Несмотря на блокировку, переменную shared_resource следует объявить как volatile.
• Перед нами создаётся и тут же уничтожается анонимный объект. Поскольку он тут же уничтожается, блокировка не происходит.
• Здесь создаётся новая переменная с именем m, инициализированная по умолчанию. Соответственно, мьютекс захвачен не будет. [ПРАВИЛЬНО]

Ответ

Автор кода ожидает, что анонимная локальная переменная типа std::unique_lock будет блокировать и разблокировать мьютекс m посредством RAII. На самом деле создаётся новая переменная с именем m, инициализированная по умолчанию. Соответственно, мьютекс захвачен не будет. См. также "Most vexing parse".

Примечание команды PVS-Studio

Очень редкая ошибка. Соответствующая диагностика V1025 в PVS-Studio есть, но пока при исследовании открытых проектов мы так и не нашли ошибку такого типа. Интересно было её встретить, как говорится, в естественной среде обитания :).

Задача N7. memcmp

А вот такой код использовался для подсчёта контрольных сумм областей в PathEngine и случайно сбоил в релизной сборке на разных компиляторах. При сборке для PlayStation использовался определённый флаг, который маскировал проблему, а на Xbox его не было. Ошибку обнаружили, когда попробовали собрать библиотеку на ПК с помощью Сlang.

struct AreaCrc32 {
  unsigned char buffType = 0;
  int crc = 0;
};

AreaCrc32 s1 {};
AreaCrc32 s2 {};

void similarArea(const AreaCrc32 &s1, const AreaCrc32 &s2) {
  if (!std::memcmp(&s1, &s2, sizeof(AreaCrc32))) {
    // ....
  }
}

Варианты ответа:

• Всё просто. Опечатка. Оператор отрицания (!) лишний.
• Проблема связана с выравниванием полей в структуре. [ПРАВИЛЬНО]
• Вычисляется размер указателя вместо размера структуры.

Ответ

Поля в структуре будут выравнены по границе в 4 байта. Между buffType и сrc образуются байты выравнивания, которые могут быть заполнены нулями, а могут мусором.

struct S {
  unsigned char buffType = 0;
  char[3] _padding = { compiler don't care about content in release };
  int сrc = 0;
};

Функция memcmp сравнивает память побайтово, захватывая и мусорные байты тоже, поэтому получается неизвестный результат этой операции при сравнении s1 с s2. Настройки компилятора для плойки явно указывали компилятору забивать байты выравнивания нулями. Правила C++ для структур говорят, что нам нужно использовать в этом месте оператор ==, и memcmp не предназначен для данной задачи.

Заодно приведём ещё один вариант, когда может сравниваться лишнее. Как-то раз на ревью было вот такое. Вовремя заметили.

class C {
  int i;
  
public:
  virtual void f();
   
  // ...
};
  
void similar(C &c1, C &c2) {
  if (!std::memcmp(&c1, &c2, sizeof(C))) {
    // ...
  }
}

Сравнивать плюсовые структуры и классы через memcmp — плохая затея, особенно если в них есть указатель на таблицу виртуальных функций.

Пусть от C унаследованы два разных класса. Тогда если c1 и c2 — ссылки на экземпляры этих двух разных классов, то они не будут равны, даже если все данные совпадают (кроме указателя на таблицу виртуальных функций).

Но это ещё не всё. Из-за наличия неявного указателя в классе могут начаться проблемы с выравниванием (наличие байт выравнивания).

Задача N8. operator =

Тут вроде просто должно быть, но что-то часто на таких вещах претенденты спотыкаются на собеседованиях.

struct S { S(const S *) noexcept; /* ... */ };
  
class T {
  int n;
  S *s1;
  
public:
  T(const T &rhs) : n(rhs.n),
                    s1(rhs.s1 ? new S(rhs.s1) : nullptr) {}
  ~T() { delete s1; }
  
  // ...
  
  T& operator=(const T &rhs) {
    n = rhs.n;
    delete s1;
    s1 = new S(rhs.s1);
    return *this;
  }
};

Варианты ответа:

• Правильнее написать return rhs;
• В operator = нет защиты, что объект присваивается сам себе.
• В operator = следует обнулить указатель s1 после delete s1;. Иначе возможно двойное освобождение памяти.
• Одновременно 2 и 3. [ПРАВИЛЬНО]
• Если это 64-битное приложение, то можно уменьшить размер структуры, поменяв местами поля n и s1. Размер уменьшится с 16 до 12 байт.

Ответ

В operator = нет проверки, что объект присваивается сам себе. В итоге удалим данные и загрузим какой-то мусор.

В operator = есть ещё одно слабое место. Если исключения в проекте не отключены, то потенциально new может бросить std::bad_alloc. В этом случае в this->s1 остаётся старое и, возможно, ненулевое значение. Которое затем будет передано в delete в деструкторе. И получим double free.

Примечание команды PVS-Studio

Переставляя поля в структуре/классе, действительно иногда можно уменьшить их размер. Но к данному классу это не относится. В 32-битной программе размер класса всегда будет 8 байт, а в 64-битной — всегда 16 байт из-за необходимости выравнивания. По крайней мере это так для наиболее распространённых моделей данных и архитектур. Подробнее про уменьшение размера классов и структур: V802.

Задача N9. Строка в DLL

Даже в четырёх строках кода можно словить неочевидный баг. Особенно если это Unity Engine, который любит включать общие заголовки в разных DLL, а потом сравнивать их вот так super_secret() == super_secret(). Казалось бы, что может пойти не так, но секрет != секрету, и игра не грузится на Windows Phone.

// header a.h
constexpr inline const char* super_secret(void) {
  constexpr const char *STRING = "string";
  return STRING;
}

// где-то в a.dll
if (super_secret() == saved_string_ptr)

// где-то в b.dll
if (super_secret() != saved_string_ptr)

Варианты ответа:

• Раз это в h-файле, то жди проблем, если использовать эту функцию в разных DLL. В каждой будет свой экземпляр строки. [ПРАВИЛЬНО]
• Функция возвращает указатель на временную строчку, и его нельзя использовать. Вернее, это приводит к неопределённому поведению.
• constexpr следует заменить на static.

Ответ

А ошибка в том, что общий хедер a.h использовался в разных библиотеках. Все работало, пока движок собирался как единая DLL. Собрав две разные DLL, мы получим два разных адреса для STRING. А вообще ошибка в том, что строки сравнивают как указатели, а не функцией strcmp.

Примечание команды PVS-Studio

Большинство описанных в статье багов так или иначе связано с индустрией разработки игр. А раз так, возможно эту статью читает немало GameDev разработчиков. У команды PVS-Studio есть просьба. Мы начали реализовывать в анализаторе диагностики для выявления багов, специфичных для проектов, основанных на Unity и Unreal Engine. Просим присылать нам идеи для новых диагностик.

Задача N10. serialize

У этой функции может быть UB с переполнением буфера. Часто это происходит для Clang с оптимизацией -O3, и не происходит с -O0. На clang 12.10 и выше это уже исправлено для всех режимов оптимизации. Код не мой, всплыл на каком-то из собесов, когда просто беседовали по душам.

char destBuffer[16];
void serialize(bool x) {
  const char* whichString = x ? "true" : "false";
  const size_t len = strlen(whichString);
  memcpy(destBuffer, whichString, len);
}

int main() {
  bool x;
  serialize(x);
  printf("%s", destBuffer);
  return 0;
}

Варианты ответа:

• Так как переменная x не инициализирована, программа в зависимости от ключей оптимизации и фазы Луны будет печатать то "true", то "false".
• Из-за неопределённого поведения может быть напечатано не только "true", "false", но ещё и пустая строка.
• Этот код не скомпилируется.
• Использование неинициализированной переменной x приводит к неопределённому поведению. Как оно себя проявит, обсуждать смысла нет. Программа может упасть, распечатать мусор, что угодно. [ПРАВИЛЬНО]

Ответ

Здесь компилятор перехитрил сам себя. Итак, что же происходит? Для начала стоит напомнить, что bool — это не просто 0 или 1, он зависит от компилятора. Clang определяет, что 0 — ложь, другое — истина. Агрессивная оптимизация компилятора преобразует вычисление len в 'len = 5 – x', а переменная x не инициализирована, поэтому в некоторых случаях у нас получается 'len = 5–7'. В таком случае программа падает при вызове memcpy из-за переполнения буфера.

Примечание команды PVS-Studio

Программисты достаточно часто ошибаются, пытаясь предсказать, как проявит себя неопределённое поведение. Это невозможно и не нужно делать. Хорошая заметка на эту тему: "Ложные представления программистов о неопределённом поведении".

Задача N11. sayHello

Что будет выведено в результате работы программы?

void sayHello() {
    std::cout << "Hello, World!\n";
}
void sayHello() {
    std::cout << "Goodbye, World!\n";
}
int main() {
    sayHello();
    return 0;
}

Варианты ответа:

• Не морочьте мне голову. Этот код не компилируется.
• Будет напечатано "Hello, World!".
• Будет напечатано "Hello, World!", а потом "Goodbye, World!". Есть такая магия.
• Тут есть один прикол, про который я уже знаю. Или не знаю, но предвижу. [ПРАВИЛЬНО]

Ответ

При запуске кода в консоль будет выведено "Goodbye, World!". Потому что имя второй функции sayHello написано с использованием Unicode, который, к сожалению, не всегда визуально различим. Она же и вызывается (Clang, latest).

sayHello():                           # @sayHello()
        push    rbp
        mov     rbp, rsp
        mov     rdi, qword ptr [rip + std::cout@GOTPCREL]
        lea     rsi, [rip + .L.str]
        call    std::basic_ostream<char, std::char_traits<char> >&
  std::operator<< <std::char_traits<char> >(std::basic_ostream<char,
  std::char_traits<char> >&, char const*)@PLT
        pop     rbp
        ret
"_Z9sayHellov":                         # @"_Z9say\D0\9Dellov"
        push    rbp
        mov     rbp, rsp
        mov     rdi, qword ptr [rip + std::cout@GOTPCREL]
        lea     rsi, [rip + .L.str.1]
        call    std::basic_ostream<char, std::char_traits<char> >&
  std::operator<< <std::char_traits<char> >(std::basic_ostream<char,
  std::char_traits<char> >&, char const*)@PLT
        pop     rbp
        ret

Примечание команды PVS-Studio

Всех, кому интересно узнать про эти нюансы, связанные с Unicode, приглашаем сюда: "Атака Trojan Source для внедрения в код изменений, незаметных для разработчика".

Задача N12. Модуль числа

Функция обладает удивительной способностью изменять реальность в зависимости от того, сколько ног у ее вызывающего.

int abs_legs(int my_legs) {
  if (my_legs < 0) {
    return -my_legs;
  }
}

Варианты ответа:

• Что тут думать-то. Отсутствует return в конце функции, который будет выполняться для неотрицательных чисел. Это приводит к неопределённому поведению. [ПРАВИЛЬНО]
• На самом деле никой беды нет, так как из-за отсутствия return код не скомпилируется.
• Всё корректно. Начиная с C++20, подразумевается, что, если нет явного return, функция с одним аргументом возвращает этот самый аргумент. Т.е. как раз функция просто по умолчанию вернёт my_legs. Прикол в том, что ранее этот код был некорректен, но починился при переходе на C++20.

Ответ

Функция, возвращающая значение, должна возвращать значение (хм!) из всех возможных веток, потому что всегда найдётся такое условие, которое приведёт к неопределённому поведению.

Правильный код:

int abs_legs(int my_legs) {
  if (my_legs < 0) {
    return -my_legs;
  }
  return my_legs;
}

Примечание команды PVS-Studio

Многие думают, что неопределённое поведение здесь сводится к тому, что функция может вернуть случайное значение. Ещё раз скажем, что это не так. С уверенностью можно говорить только что этот код содержит ошибку. Как именно она проявит себя, предсказать невозможно. Например, компилятор может вообще не сгенерировать инструкцию для выхода из функции. Не верите? Тогда читайте "Пример проявления неопределённого поведения из-за отсутствия return".

Задача N13. Где деньги, Лебовски?

Эта функция не раскрывается полностью, оставляя свою магию за покровом компилятора.

int get_money(int index, const int *pockets) {
  int a = index + pockets[++index];
  // ....
  return a;
}

Варианты ответа:

• Перед нами неуточнённое поведение.
• Перед нами неопределённое поведение. [ПРАВИЛЬНО]
• Начиная с C++17, этот код корректен, и известно, что он вычислит.

Ответ

Компилятор может переставить порядок операции для index + pockets[++index], и это приведёт к неоднозначному поведению с разными настройками оптимизации. Неупорядоченная или неопределённая последовательность операций приведёт к сайд эффекту при работе с переменной index.

Аналогичный код, приведённый в "Order of evaluation", считается приводящим именно к неопределённыму поведению:

n = ++i + i;      // undefined behavior

Так что следует переписать код так:

int a = index + pockets[index + 1];
++index;

Или так, в зависимости от того, что хочется получить:

++index;
int a = index + pockets[index];

Примечание команды PVS-Studio

А вы знаете, как поведёт себя следующий код?

printf("%d, %d\n", i++, i++);

Уверены? Приглашаем почитать "Глубина кроличьей норы или собеседование по C++ в компании PVS-Studio".

Задача N14. cin

С виду корректный код, но ведь не просто так он попал сюда? :)

void umcp_read_buffer_from_pipe() {
  char bufKernel[12];
  char bufMatrix[12];
  std::cin.width(12);
  std::cin >> bufKernel;
  std::cin >> bufMatrix;
}

Варианты ответа:

• Нужен ещё один вызов std::cin.width(12).
• Код корректно работает, начиная с C++20.
• Верны 1 и 2. [ПРАВИЛЬНО]
• Это корректный код. Просто пример для передышки :)
• Правильно будет написать std::cin.width(12-1); чтобы учесть терминальный ноль.
• Этот код, независимо от вводимых данных, приводит к неопределённому поведению.

Ответ

В этом примере первое чтение не приведёт к переполнению и заполнит bufKernel усечённой строкой. Но второе чтение может переполнить bufMatrix. Чтобы этого не произошло, надо также вызвать std::cin.width(12); перед получением bufMatrix. Или воспользоваться безопасной работой через строки.

void umcp_read_buffer_from_pipe() {
  std::string bufKernel, bufMatrix;
  std::cin >> bufKernel >> bufMatrix;
}

Если вы используете C++20, то будет выбрана следующая перегрузка функции, и всё пройдёт безопасно. Даже если вы специально об этом не заботились...

template< class Traits, std::size_t N >
basic_istream<char, Traits>&
  operator>>( basic_istream<char, Traits>& st, signed char (&s)[N] );

Задача N15. display_string

Кажется, в этом коде "нильзя-программер" просто смешивает случайности с абсурдом и называет это "настройкой отображения".

std::string str_func();
void display_string(const char *);

void set_display_options() {
  const char *str = str_func().c_str();
  display_string(str);
}

Варианты ответа:

• Временный объект строки уничтожается, и после этого указатель на буфер использовать нельзя. [ПРАВИЛЬНО]
• Этот код не компилируется, начиная с C++11.
• Если строка пустая, то указатель str окажется равен nullptr.

Ответ

Здесь std::string::c_str вызывается для временного объекта std::string. Полученный указатель будет указывать на освобожденную, но, возможно, еще валидную память после уничтожения объекта std::string в конце выражения присваивания. Однако надеяться на это не стоит – при разыменовании этого указателя получите неопределённое поведение.

Правильно:

std::string str = str_func();
const char *cstr = str.c_str();
display_string(cstr);

Или так:

display_string(str_func().c_str())

Задача N16. Бар

Почему утром опасно ходить в bar?

void morning(const std::string &owner) {
  std::fstream bar(owner);
  if (!bar.is_open()) {
    // Handle error
    return;
  }

  bar << "customer";

  std::string str;
  bar >> str;
}

Варианты ответа:

• Всё хорошо. Запишем "customer" и прочитаем тоже самое.
• Мы прочитаем из потока "пустоту". [ПРАВИЛЬНО]
• Неправильно реализована проверка, что удалось открыть поток.
• Код опасен, если включить режим оптимизации -O2.

Ответ

Потому что можно не выйти. Данные добавляются в конец файла, а затем считываются из того же файла. Однако поскольку указатель в файле остался на прежнем месте (в конце), то попытка прочитать данные (из конца) файла ни к чему не приведёт. В бар надо ходить вечером, тогда всё "замечательно выходит" (с).

void evening(const std::string &owner) {
  std::fstream bar(owner);
  if (!bar.is_open()) {
    // Handle error
    return;
  }

  bar << "customer";

  std::string str;
  bar.seekg(0, std::ios::beg);
  bar >> str;
}

Задача N17. Бард

Станет ли бард пьяницей?

class Bard {
  int _beer;
  int _meal;

public:
  Bard(int meal) : _meal(meal), _beer(_meal - 1) {}
};

Варианты ответа:

• Поля классов нельзя начинать с подчёркивания. Это приводит к неожиданным последствиям при использовании некоторых компиляторов.
• Для инициализации _beer используется ещё неинициализированная переменная _meal. [ПРАВИЛЬНО]
• Всё правильно.
• Всё правильно, при условии, что от этого класса нельзя наследоваться.

Ответ

Члены класса инициализируются в порядке их объявления, а не в том, как они перечислены в списке инициализации конструктора.

Порядок инициализации членов класса нарушен. Из-за этого сначала будет инициализирована переменная _beer, а уже затем _meal. Попытка прочитать значение _meal до его инициализации приводит к неопределённому поведению. Скорее всего, бард сопьётся без закуси.

Исправить можно так:

Bard(int meal) : _beer(meal - 1), _meal(meal) {}

Другой вариант исправления заключается в использовании везде для инициализации переменной meal:

Bard(int meal) : _meal(meal), _beer(meal - 1) {}

Ещё можно поменять поля класса местами:

class Bard {
  int _meal;
  int _beer;

public:
  Bard(int meal) : _meal(meal), _beer(_meal - 1) {}
};

Задача N18. Итераторы

В каком случае выстрел по ногам будет особенно результативным?

std::vector<....> formats;
....
for (auto format = begin(formats), __end = end(formats);
     format != __end; ++format)
{
  if (snd::CodecNamesEq(....)) {
    format->is_stereo = true;
    formats.push_back(stereo_format);
  }
}

Варианты ответа:

• Имена переменных не должны содержать два идущих подряд подчёркивания.
• После вызова push_back итераторы станут невалидными.
• Нужно писать format++, а не ++format. В данном случае это важно.
• Одновременно 1 и 2. [ПРАВИЛЬНО]
• Одновременно 1, 2 и 3.

Ответ

Если в процессе push_back произойдет реаллокация внутри контейнера formats, то итератор format будет указывать уже на удалённую память, работа с которой вызовет кучу проблем.

И переменная с именем __end — это плохо. Переменные, содержащие два знака подчёркивания подряд, зарезервированы для стандартной библиотеки и внутреннего использования.

Примечание. Хоть это и не указано в коде, но подразумевается, что используемый контейнер может инвалидировать итераторы при вставке элементов в конец. Это важно, т.к. тот же push_back у std::list так не делает.

Всем спасибо

Спасибо Сергею Кушниренко за предоставленные материалы для создания квиза и этой публикации. Спасибо Андрею Карпову, Михаилу Гельвиху, Филиппу Хандельянцу и другим членам команды PVS-Studio. Почему бы, собственно, самих себя не похвалить :). Спасибо читателям, надеемся, это было интересно. Если хочется ещё, приглашаем сюда.

Оставляйте комментарии про свои забавные случаи или странный код, который вы когда-то повстречали.

Дополнительные ссылки

• Статьи и их обсуждение, на основе которых был создан этот квиз: Федя, дичь, Сказка про собес наоборот.
• Обсуждение некоторых приведённых здесь заданий на сайте Reddit: Hello, World! Where are your bugs?
• Многие из описанных здесь ошибок могут быть найдены с помощью анализатора PVS-Studio. Чтобы не загромождать текст, мы не стали разбирать, что может найти анализатор, а что нет и почему. Однако если вы подозреваете что в вашем коде потенциально могут быть подобные неожиданные проблемы, то предлагаем скачать его и попробовать.