Toп 10 ошибок в C++ проектах за 2017 год

[datacompboy]

М--даааа… Это что, кто-то #define true false забабахал уходя?!
Как, ну КААК иначе такие дефайны появляются, блин?

[vanxant]

Напомнило недавнее из javascript

function if (x) {return true;}

Здесь после if неразрывный пробел  , который будет считаться частью идентификатора, плюс любовь js к автоматической расстановке точек с запятой, что позволяет этому работать.
Конечно, не так злобно, как в примере в статье, зато если хочется нагадить — можно применять избирательно, только по особо сложным местам.

[vagran]

Пример с CryEngine некорректен. Там, очевидно, всё правильно — новый элемент добавляется в конец односвязного списка. Цикл ищет последний элемент, довольно рапространённый паттерн. Предложенное вами «исправление» не имеет смысла.

[SergVasiliev]

Только я не предлагал 'исправлений'. :)
Возможно, недостоточно ясно выразился в статье.

В коде CryEngine форматирование поправили, к слову.

[MarkGrin]

Если всё правильно, то форматирование плохое. Отступ намекает, что оператор принадлежит циклу, а если всё правильно, то это не так.

[MooNDeaR]

Больше половины ошибок GCC выдаст в режиме -Wall -Wextra.

Ошибку с sprintf, я думаю, он тоже покажет. Да даже если не покажет, то тут уж надо разуть глаза и посмотреть на код в IDE. Макросы подсвечиваются другим цветом, нежели функции. Ну, а если ты сделал одним цветом, то, как обычно, ССЗБ.

[Andrey2008]

Прошу продемонстрировать. Очень часто слышу про "-Wall -Wextra", но никто из авторов не удосуживается попробовать проверить свои предположения про магическую силу gcc/clang :).

[valexey]

godbolt.org/g/4p3ZGg

-Wall и -Wextra не помогают конечно, но помогает -Weverything

[Andrey2008]

Хорошо, одна есть. Что касаемо «больше половины ошибок GCC выдаст в режиме ....»? :)

[valexey]

Это не моё утверждение. Я просто показал, что, при желании, компилятор кое-что может отловить.

Естественно тюнинг флагов компилятора под проект (чтобы не втыкать -Weverything) — это отдельная задача.

[LynXzp]

На if(); gcc ругается, а вот на for(); нет. Да, не много чего он ловит, но и на том спасибо (у меня была такая ошибка, а еще if(a=b)).

[valexey]

Можно не только уточнить, но и самостоятельно проверить. Заодно и сравнить с тем, сколько на указанных исходниках будет сообщений от, скажем, PVS-Studio, Clang Static Analyzer.

Любопытно будет прочитать про результаты такого теста и сравнения.

[DinoL]

У второго места (ошибка в Unreal Engine) ещё перепутаны местами два последних аргумента в вызове, судя по их названиям. Впрочем, на работу функции это влияния не оказывает.

Объявление и вызов соответственно:
static void CreateProjectSet(… int32 OutCreatedProjects, int32 OutMatchedProjects);
GameProjectAutomationUtils::CreateProjectSet(… OutMatchedProjectsMob, OutCreatedProjectsMob);

[Mr_Rm]

А в примере из FreeBSD (седьмое место) действительно «повезло», что где-то выше объявлена переменная out, и вот эта строка

mlx5_cmd_exec(dev, &din, sizeof(din), &out, sizeof(dout));

компилируется без ошибки?

[VokaMut]

Тогда прошу вас выкинуть всю технику из дома имеющую больше 3 кнопок, ведь её мозги тоже написаны на крестах

[xenohunter]

В коде, который на пятом месте, IDE должна была подсветить неиспользуемую переменную. И банальный линтер в pre-commit hook выловил бы такое. Хорошо хоть, что разработчики решили использовать статический анализатор (как я понимаю, постфактум, то есть, на коде из репозитория).

[Andrey2008]

for(int i = 0; i < SelectedObjects.Num(); ++i)
{
  UObject* Obj = SelectedObjects[0].Get();
  EdObj = Cast<UEditorSkeletonNotifyObj>(Obj);
  if(EdObj)
  {
    break;
  }
}

А где здесь неиспользуемая переменная?

[xenohunter]

Извиняюсь, протупил: i не используется только в теле цикла, но используется в объявлении.

[3aicheg]

Шутка с «не переходите к описанию проблемы», всё же, не очень хороша — кто-нибудь с хорошим знанием синтаксиса С++ может и спятить, как географ, не нашедший на карте Берингова пролива.

[nexmean]

И после этого кто-то говорит о том, что Rust не нужен… 8 из 10 таких ошибок в Rust просто не скомпилируются.

[Antervis]

8 из 10 приведенных участков написаны на диалекте с++, который старше раста

[nexmean]

То есть в современном C++ компиляция данного кода упадёт с ошибкой? Или вы имеете ввиду "старость надо уважать"?

[domix32]

Может и не собраться, да. Но суть скорее в размере кодовой базы которую нельзя просто взять и RIIRнуть

[Antervis]

сегодня на с++ так уже не пишут, а когда писали — не было раста. Код на современном с++ в свою очередь менее подвержен ошибкам.

[viktprog]

Не согласен по поводу CryEngine.

  for(pmd0=m_pMeshUpdate; pmd0->next; pmd0=pmd0->next);
    pmd0->next = pmd;

Очевидно тут ищется конец списка и вставляется новый элемент в конец. В противном случае получится бесконечный цикл, поскольку после первого прохода pmd0 = pmd0->next <=> pmd0 = pmd, затем второй проход в теле цикла pmd0->next = pmd <=> pmd->next = pmd, привет infinite loop.

Так что это баг автоматического форматирования, а не программиста.

[SergVasiliev]

Выше обсуждали.

[AKudinov]

Наверное, я отстал от моды, но как такое возможно?

sprintf — это макрос, который раскрывается в (!) std::printf!

Вот пишут, что, как и раньше,
int sprintf ( char * str, const char * format,… );

Иначе весь «legacy»-код можно выбросить на помойку. Разве нет?

[AKudinov]

Разумеется.
Но кто, где и зачем такое написал, мне непонятно?
Или это где-то в том же исходнике было, но автор умолчал?
Что-то в стиле
#define TRUE FALSE

[SergVasiliev]

Это define из исходников.
Мне показалось это очевидным, поэтому я не стал писать этот факт прямым текстом.

[AKudinov]

Подозреваю, что в примере с mlx5_core_create_qp() просто опечатка при вызове mlx5_cmd_exec(): если предпоследний аргумент &dout, всё встаёт на свои места (и sizeof(dout)). Иначе зачем dout, вообще, нужен?
Приведённого куска кода явно недостаточно, чтобы сделать правильный вывод…

[Deranged]

mlx5_core_create_qp()…

т.к. подобная оптимизация не влияет на поведение программы с точки зрения C/C++

Да ну, не может быть. Откуда компилятору знать, что memset не имеет побочных эффектов? Это можно понять только в рантайме. А если memset'у скормить адрес переменной из стека, а размер указать побольше, что бы записалась и следующая? А её уже использовать в коде? Понятно, что это страшный костыль, и того кто так делает к клавиатуре вообще подпускать нельзя, но тем не менее, так сделать можно. И компилятор не имеет права предполагать отстутствие побочных эффектов.

[Gumanoid]

Это будет undefined behavior.

[Deranged]

Ну behavior, к сожалению, вполне себе defined, если известна целевая архитектура, а компилятору она известна. Иначе не работали бы так любимые всеми атаки на переполнение стекового буфера. Но не суть. Даже если и undefined — это все равно побочный эффект. А раз вызов функции может иметь побочные эффекты — удалять вызов нельзя. Компилятор может определить отсутствие побочных эффектов по синтаксическому дереву функции. Очевидно, что если, например, это const функция, внутри которой нет ничего подозрительного, типа обращения к mutable членам, или вызова внешних функций, то вызов можно вырезать. Но в случае memset, очевидно, присутствуют команды записи данных по адресу. Что бы понять, что такую запись можно удалить, компилятор по сути должен понять, что не важно, была сделана запись, или нет, программа дальше будет работать одинаково. Т.е. надо понимать, что после записи, этот адрес обязательно будет еще раз перезаписан, до попытки повторно из него прочитать. А еще есть такие вещи, как отображаемые в память файлы, порты ввода вывода. Т.е. если есть только код, который в память только пишет, но нет кода, который из неё обратно читает, это еще не значит, что запись можно не делать. Задача определения отсутствия побочных эффектов при наличии операций записи по адресу — очень крутая, в complie time её решить крайне трудно.

[Gumanoid]

Это не так работает. Компилятор делает оптимизации исходя из того, что код написан правильно и UB не может возникнуть ни при каких входных данных.
То есть если написан memset локальной переменной, которая потом не читается, а размер приходит извне, то такой memset можно удалить.

[monah_tuk]

Пардон, но:

#define sprintf std::printf

это в чьём коде? Без этих данных решить задачу нет возможности. Тем более, что в Gcc актуальных версий такого определения в cstdio нет, там сделано так:

#undef sprintf
namespace std
{
  using ::sprintf;
}

[Antervis]

видимо, этот дефайн был добавлен для отладки и его забыли убрать

[Andrey2008]

Зачем фантазировать… В статье же есть отсылка к первоисточнику: "Любите статический анализ кода". Там показано, что это просто такой дурацкий стиль именования функций, с помощью буквы 's'. Примеры: #define sfstream std::fstream. Злополучный заголовочный файл: definesTypes.h.

[monah_tuk]

Просто если лезть в статью, то как быть с пожеланием:

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

;-)