![Picture 1](https://habrastorage.org/getpro/habr/post_images/c35/365/51c/c3536551c107bb033939d15ce93397eb.png)
За окном уже почти как 3 месяца стоит 2018 год, а это значит, что пришло время (пусть и немного запоздало) составить топ 10 ошибок, найденных анализатором PVS-Studio в C++ проектах за прошедший год. Итак, начнём!
Примечание. Для большего интереса я рекомендую вам сначала попробовать самостоятельно найти ошибки в приведённых фрагментах кода, и только после этого читать предупреждение анализатора и пояснения. Думаю, так будет намного интереснее.
Десятое место
Источник: Notepad++: проверка кода пять лет спустя
Ошибка была обнаружена при проверке одного из самых известных текстовых редакторов — Notepad++.
Фрагмент кода, содержащий ошибку:
TCHAR GetASCII(WPARAM wParam, LPARAM lParam)
{
int returnvalue;
TCHAR mbuffer[100];
int result;
BYTE keys[256];
WORD dwReturnedValue;
GetKeyboardState(keys);
result = ToAscii(static_cast<UINT>(wParam),
(lParam >> 16) && 0xff, keys, &dwReturnedValue, 0);
returnvalue = (TCHAR) dwReturnedValue;
if(returnvalue < 0){returnvalue = 0;}
wsprintf(mbuffer, TEXT("return value = %d"), returnvalue);
if(result!=1){returnvalue = 0;}
return (TCHAR)returnvalue;
}
Предупреждение PVS-Studio: V560 A part of conditional expression is always true: 0xff. babygrid.cpp 711
Анализатор счёл подозрительным выражение (lParam >> 16) && 0xff. Второй аргумент, передаваемый в функцию ToAscii, всегда будет иметь значение 0 или 1, причём результирующее значение зависит только от левого подвыражения — (lParam >> 16). Очевидно, что вместо оператора && должен был использоваться оператор &.
Девятое место
Источник: Передаю привет разработчикам компании Yandex
На девятом месте расположилась ошибка из проекта ClickHouse, разрабатываемого компанией Yandex.
bool executeForNullThenElse(....)
{
....
const ColumnUInt8 * cond_col =
typeid_cast<const ColumnUInt8 *>(arg_cond.column.get());
....
if (cond_col)
{
....
}
else if (cond_const_col)
{
....
}
else
throw Exception(
"Illegal column " + cond_col->getName() +
" of first argument of function " + getName() +
". Must be ColumnUInt8 or ColumnConstUInt8.",
ErrorCodes::ILLEGAL_COLUMN);
....
}
Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'cond_col' might take place. FunctionsConditional.h 765
В данном коде неправильно обрабатывается ошибочная ситуация, когда необходимо сгенерировать исключение. Обратите внимание на указатель cond_col. Для него в операторе if выполняется проверка, что указатель — ненулевой. Если управление дошло до else ветви, где генерируется исключение, указатель cond_col точно нулевой. Однако при формировании сообщения исключения, cond_col разыменовывается в выражении cond_col->getName().
Восьмое место
Источник: Сравнение качества кода Firebird, MySQL и PostgreSQL
На восьмом месте расположилась одна из ошибок, найденных в проекте MySQL во время сравнения качества кода Firebird, MySQL и PostgreSQL.
Код метода, содержащего ошибку:
mysqlx::XProtocol* active()
{
if (!active_connection)
std::runtime_error("no active session");
return active_connection.get();
}
Предупреждение PVS-Studio: V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw runtime_error(FOO); mysqlxtest.cc 509
В случае отсутствия активного соединения (!active_connection) создаётся объект исключения типа std::runtime_error и… на этом всё. После создания он будет попросту удалён, при этом продолжится выполнение метода. Очевидно, что разработчик забыл ключевое слово throw, чтобы сгенерировать исключение.
Седьмое место
Источник: Как найти 56 потенциальных уязвимостей в коде FreeBSD за один вечер
Как найти 56 потенциальных уязвимостей за вечер? С помощью статического анализа, конечно же!
Одна из проблем, обнаруженных в коде FreeBSD:
int mlx5_core_create_qp(struct mlx5_core_dev *dev,
struct mlx5_core_qp *qp,
struct mlx5_create_qp_mbox_in *in,
int inlen)
{
....
struct mlx5_destroy_qp_mbox_out dout;
....
err_cmd:
memset(&din, 0, sizeof(din));
memset(&dout, 0, sizeof(dout));
din.hdr.opcode = cpu_to_be16(MLX5_CMD_OP_DESTROY_QP);
din.qpn = cpu_to_be32(qp->qpn);
mlx5_cmd_exec(dev, &din, sizeof(din), &out, sizeof(dout));
return err;
}
Предупреждение PVS-Studio: V597 The compiler could delete the 'memset' function call, which is used to flush 'dout' object. The memset_s() function should be used to erase the private data. mlx5_qp.c 159
Обратите внимание на выражение memset(&dout, 0, sizeof(dout)). Разработчик хотел «затереть» данные в блоке памяти, соответствующем dout, выставив нулевые значения. Обычно такой подход используется, когда необходимо очистить какие-то приватные данные, чтобы они не «висели» в памяти.
Однако дальше dout не используется (sizeof(dout) не в счёт), что позволит компилятору удалить приведённый выше вызов функции memset, т.к. подобная оптимизация не влияет на поведение программы с точки зрения C/C++. Как следствие — данные, которые должны были быть зачищены, могут так и остаться в памяти.
Чтобы глубже погрузиться в тему, я рекомендую для прочтения следующие статьи:
- Безопасная очистка приватных данных.
- Документация к диагностическому правилу V597.
- Самая опасная функция в мире С/С++.
Шестое место
Источник: Долгожданная проверка CryEngine V
Первый игровой движок, к коду которого мы обратимся в этом топе — CryEngine V.
int CTriMesh::Slice(....)
{
....
bop_meshupdate *pmd = new bop_meshupdate, *pmd0;
pmd->pMesh[0]=pmd->pMesh[1] = this; AddRef();AddRef();
for(pmd0=m_pMeshUpdate; pmd0->next; pmd0=pmd0->next);
pmd0->next = pmd;
....
}
Предупреждение PVS-Studio: V529 Odd semicolon ';' after 'for' operator. boolean3d.cpp 1314
Согласитесь, что если бы этот фрагмент не был выписан вот так вот — будучи сокращённым и отделённым от остального кода, не так легко было бы обнаружить в нём тот подозрительный участок, который нашёл анализатор — символ ';', завершающий цикл for. При этом форматирование кода (сдвиг следующего выражения) также наводит на мысли о том, что символ ';' тут лишний, а выражение pmd0->next = pmd; должно быть телом цикла. Но судя по логике цикла for, здесь именно неправильное форматирование кода, сбивающее с толку, а не логическая ошибка. В коде CryEngine форматирование кода, кстати, поправили.
Пятое место
Источник: Статический анализ как часть процесса разработки Unreal Engine
Следующая ошибка была обнаружена в ходе работы над исправлением ошибок, найденных PVS-Studio в коде игрового движка Unreal Engine.
for(int i = 0; i < SelectedObjects.Num(); ++i)
{
UObject* Obj = SelectedObjects[0].Get();
EdObj = Cast<UEditorSkeletonNotifyObj>(Obj);
if(EdObj)
{
break;
}
}
Предупреждение PVS-Studio: V767 Suspicious access to element of 'SelectedObjects' array by a constant index inside a loop. skeletonnotifydetails.cpp 38
В цикле хотели обойти все элементы и найти среди них первый, имеющий тип UEditorSkeletonNotifyObj. Но допустили досадную опечатку, использовав в выражении SelectedObjects[0].Get() константный индекс 0 вместо счётчика цикла i. Как результат — всегда будет проверяться только первый элемент.
Четвёртое место
Источник: 27000 ошибок в операционной системе Tizen
Ошибка была найдена при проверке операционной системы Tizen, а также сторонних компонентов, используемых в ней. Статья большая, и в ней выписано много интересных примеров ошибок — настоятельно рекомендую ознакомиться.
Но вернёмся к конкретному предупреждению:
int _read_request_body(http_transaction_h http_transaction,
char **body)
{
....
*body = realloc(*body, new_len + 1);
....
memcpy(*body + curr_len, ptr, body_size);
body[new_len] = '\0';
curr_len = new_len;
....
}
Предупреждение PVS-Studio: V527 It is odd that the '\0' value is assigned to 'char' type pointer. Probably meant: *body[new_len] = '\0'. http_request.c 370
Ошибка кроется в выражении body[new_len] = '\0'. Обратите внимание, что параметр body имеет тип char**, соответственно, тип выражения body[new_len] — char*. Но разработчик дал маху, и, забыв ещё одно разыменование, попытался записать в указатель значение '\0' (будет преобразовано в нулевой указатель).
Это приводит к двум проблемам:
- Куда-то (body[new_len]) будет записан нулевой указатель.
- Терминальный ноль не будет записан в конец строки.
Корректный код:
(*body)[new_len] = '\0';
Третье место
Источник: Как PVS-Studio может помочь в поиске уязвимостей?
Вот мы и подобрались к тройке лидеров. Приведённый ниже код был обнаружен в ходе поиска ответа на вопрос: «Как PVS-Studio справляется с поиском CVE?» (ответ смотрите в указанной выше статье). Код из проекта illumos-gate.
static int devzvol_readdir(....)
{
....
char *ptr;
....
ptr = strchr(ptr + 1, '/') + 1;
rw_exit(&sdvp->sdev_contents);
sdev_iter_datasets(dvp, ZFS_IOC_DATASET_LIST_NEXT, ptr);
....
}
Предупреждение PVS-Studio: V769 The 'strchr(ptr + 1, '/')' pointer in the 'strchr(ptr + 1, '/') + 1' expression could be nullptr. In such case, resulting value will be senseless and it should not be used.
Функция strchr возвращает указатель на первое вхождение символа, заданного вторым аргументом, в строке, заданной первым аргументом. Если же такого символа не находится, strchr возвращает NULL. Однако этот факт не учитывается, и к возвращаемому значению всегда прибавляется значение '1'. Как результат, указатель ptr всегда будет ненулевым, а значит, дальнейшие проверки вида ptr != NULL не дадут информации о валидности указателя. В итоге, при определённых условиях данный код приводил к возникновению kernel panic.
Данной ошибке был присвоен идентификатор CVE-2014-9491: The devzvol_readdir function in illumos does not check the return value of a strchr call, which allows remote attackers to cause a denial of service (NULL pointer dereference and panic) via unspecified vectors.
Несмотря на то, что сама CVE была обнаружена в 2014 году, в ходе собственных исследований мы обнаружили эту ошибку в 2017, поэтому она попала в этот топ.
Второе место
Источник: Статический анализ как часть процесса разработки Unreal Engine
Ошибка, расположившаяся на втором месте, была обнаружена… да, опять в Unreal Engine. Мне она показалась очень интересной, так что я не смог удержаться и не выписать её.
Примечание. На самом деле, я бы выписал ещё парочку ошибок из приведённой выше статьи про Unreal Engine, но всё же не хочется так часто обращаться к одному и тому же проекту. Поэтому я настоятельно рекомендую посмотреть упомянутую выше статью самостоятельно, в частности — предупреждения V714 и V709.
Дальше будет много кода, но он необходим, чтобы понять суть проблемы.
bool FCreateBPTemplateProjectAutomationTests::RunTest(
const FString& Parameters)
{
TSharedPtr<SNewProjectWizard> NewProjectWizard;
NewProjectWizard = SNew(SNewProjectWizard);
TMap<FName, TArray<TSharedPtr<FTemplateItem>> >& Templates =
NewProjectWizard->FindTemplateProjects();
int32 OutMatchedProjectsDesk = 0;
int32 OutCreatedProjectsDesk = 0;
GameProjectAutomationUtils::CreateProjectSet(Templates,
EHardwareClass::Desktop,
EGraphicsPreset::Maximum,
EContentSourceCategory::BlueprintFeature,
false,
OutMatchedProjectsDesk,
OutCreatedProjectsDesk);
int32 OutMatchedProjectsMob = 0;
int32 OutCreatedProjectsMob = 0;
GameProjectAutomationUtils::CreateProjectSet(Templates,
EHardwareClass::Mobile,
EGraphicsPreset::Maximum,
EContentSourceCategory::BlueprintFeature,
false,
OutMatchedProjectsMob,
OutCreatedProjectsMob);
return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) &&
( OutMatchedProjectsMob == OutCreatedProjectsMob );
}
Отметим следующий важный момент, необходимый для понимания проблемы. Переменные OutMatchedProjectsDesk, OutCreatedProjectsDesk и OutMatchedProjectsMob, OutCreatedProjectsMob при объявлении инициализируются нулями, после чего передаются в качестве аргументов методу CreateProjectSet.
После этого переменные сравниваются в выражении оператора return. Следовательно, метод CreateProjectSet должен выполнять инициализацию последних двух аргументов.
Теперь же обратимся к методу CreateProjectSet, который и содержит ошибки.
static void CreateProjectSet(.... int32 OutCreatedProjects,
int32 OutMatchedProjects)
{
....
OutCreatedProjects = 0;
OutMatchedProjects = 0;
....
OutMatchedProjects++;
....
OutCreatedProjects++;
....
}
Предупреждения PVS-Studio:
- V763 Parameter 'OutCreatedProjects' is always rewritten in function body before being used. gameprojectautomationtests.cpp 88
- V763 Parameter 'OutMatchedProjects' is always rewritten in function body before being used. gameprojectautomationtests.cpp 89
Параметры OutCreatedProjects и OutMatchedProjects забыли сделать ссылками, и в итоге значения соответствующих аргументов просто копируются. Как результат — возвращаемое значение метода RunTest, приведённого выше, всегда true, так как все сравниваемые переменные имеют одно и то же значение, заданное при инициализации — 0.
Правильный вариант кода:
static void CreateProjectSet(.... int32 &OutCreatedProjects,
int32 &OutMatchedProjects)
Первое место
Источник: Любите статический анализ кода!
Как только я увидел эту ошибку, у меня не осталось ни единого сомнения по поводу того, кто должен возглавить топ. В общем, смотрите сами. Ни за что не переходите к описанию проблемы, пока не найдёте ошибку в приведённом фрагменте кода. Кстати, проект — StarEngine — опять является игровым движком.
PUGI__FN bool set_value_convert(
char_t*& dest,
uintptr_t& header,
uintptr_t header_mask,
int value)
{
char buf[128];
sprintf(buf, "%d", value);
return set_value_buffer(dest, header, header_mask, buf);
}
Ну что, как успехи с поиском ошибки? :)
Предупреждение PVS-Studio: V614 Uninitialized buffer 'buf' used. Consider checking the first actual argument of the 'printf' function. pugixml.cpp 3362
Наверняка у вас возник вопрос: "printf? Откуда в предупреждении анализатора printf, если в коде есть вызов только функции sprintf?"
В этом-то и суть! sprintf — это макрос, который раскрывается в (!) std::printf!
#define sprintf std::printf
В итоге неинициализированный буфер buf используется как строка форматирования. Здорово, не так ли? Я думаю, эта ошибка вполне себе заслуженно заняла первое место.
Ссылка на заголовочный файл с объявлением макроса.
Заключение
Надеюсь, вам понравились собранные ошибки. Лично мне они показались достаточно интересными. Но, конечно, ваше видение может отличаться от моего, поэтому вы можете составить свой «Tоп 10...», почитав статьи из нашего блога или посмотрев список ошибок, найденных PVS-Studio в open source проектах.
![Picture 2](https://habrastorage.org/getpro/habr/post_images/b23/00f/dce/b2300fdce779544407aedc511b3f8d12.png)
Также напоминаю, что все приведённые в статье ошибки (а также множество других) были найдены с помощью анализатора PVS-Studio, который советую попробовать и на вашем проекте: ссылка на страницу загрузки.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Vasiliev. Top 10 Bugs in the C++ Projects of 2017
Комментарии (46)
vagran
26.03.2018 12:52Пример с CryEngine некорректен. Там, очевидно, всё правильно — новый элемент добавляется в конец односвязного списка. Цикл ищет последний элемент, довольно рапространённый паттерн. Предложенное вами «исправление» не имеет смысла.
foto_shooter Автор
26.03.2018 13:09+1Только я не предлагал 'исправлений'. :)
Возможно, недостоточно ясно выразился в статье.
В коде CryEngine форматирование поправили, к слову.
MarkGrin
26.03.2018 13:28Если всё правильно, то форматирование плохое. Отступ намекает, что оператор принадлежит циклу, а если всё правильно, то это не так.
MooNDeaR
26.03.2018 14:01+1Больше половины ошибок GCC выдаст в режиме -Wall -Wextra.
Ошибку с sprintf, я думаю, он тоже покажет. Да даже если не покажет, то тут уж надо разуть глаза и посмотреть на код в IDE. Макросы подсвечиваются другим цветом, нежели функции. Ну, а если ты сделал одним цветом, то, как обычно, ССЗБ.Andrey2008
26.03.2018 14:34Прошу продемонстрировать. Очень часто слышу про "-Wall -Wextra", но никто из авторов не удосуживается попробовать проверить свои предположения про магическую силу gcc/clang :).
valexey
26.03.2018 22:21+1godbolt.org/g/4p3ZGg
-Wall и -Wextra не помогают конечно, но помогает -WeverythingAndrey2008
26.03.2018 23:34+1Хорошо, одна есть. Что касаемо «больше половины ошибок GCC выдаст в режиме ....»? :)
valexey
27.03.2018 14:14Это не моё утверждение. Я просто показал, что, при желании, компилятор кое-что может отловить.
Естественно тюнинг флагов компилятора под проект (чтобы не втыкать -Weverything) — это отдельная задача.
LynXzp
27.03.2018 14:47На if(); gcc ругается, а вот на for(); нет. Да, не много чего он ловит, но и на том спасибо (у меня была такая ошибка, а еще if(a=b)).
Am0ralist
27.03.2018 00:35Можно уточнить, а какое количество всего варнингов данный режим компилирования выдаст в сумме для протестированных исходников, чтоб реально понимать, можно ли там человеку будет увидеть конкретно эту ошибку?
valexey
27.03.2018 14:25-1Можно не только уточнить, но и самостоятельно проверить. Заодно и сравнить с тем, сколько на указанных исходниках будет сообщений от, скажем, PVS-Studio, Clang Static Analyzer.
Любопытно будет прочитать про результаты такого теста и сравнения.
DinoL
26.03.2018 14:50У второго места (ошибка в Unreal Engine) ещё перепутаны местами два последних аргумента в вызове, судя по их названиям. Впрочем, на работу функции это влияния не оказывает.
Объявление и вызов соответственно:
static void CreateProjectSet(… int32 OutCreatedProjects, int32 OutMatchedProjects);
GameProjectAutomationUtils::CreateProjectSet(… OutMatchedProjectsMob, OutCreatedProjectsMob);
Taras-proger
26.03.2018 14:50Логическое И c маской да ещё после сдвига? Что серьзно? Даже не знаю, что это может значить.
Mr_Rm
26.03.2018 15:15А в примере из FreeBSD (седьмое место) действительно «повезло», что где-то выше объявлена переменная out, и вот эта строка
mlx5_cmd_exec(dev, &din, sizeof(din), &out, sizeof(dout));
компилируется без ошибки?
xenohunter
26.03.2018 19:35В коде, который на пятом месте, IDE должна была подсветить неиспользуемую переменную. И банальный линтер в pre-commit hook выловил бы такое. Хорошо хоть, что разработчики решили использовать статический анализатор (как я понимаю, постфактум, то есть, на коде из репозитория).
Andrey2008
26.03.2018 19:38for(int i = 0; i < SelectedObjects.Num(); ++i) { UObject* Obj = SelectedObjects[0].Get(); EdObj = Cast<UEditorSkeletonNotifyObj>(Obj); if(EdObj) { break; } }
А где здесь неиспользуемая переменная?xenohunter
27.03.2018 12:46Извиняюсь, протупил:
i
не используется только в теле цикла, но используется в объявлении.
kloppspb
26.03.2018 19:52executeForNullThenElse
На первый взгляд какой-то вывих мозга. Но полез в гит… И мозг свихнулся окончательно :) Особенно вот тут:
if (cond_col) { if (!( executeLeftType<UInt8>(cond_col, block, arguments, result) || executeLeftType<UInt16>(cond_col, block, arguments, result) || executeLeftType<UInt32>(cond_col, block, arguments, result) || executeLeftType<UInt64>(cond_col, block, arguments, result) || executeLeftType<Int8>(cond_col, block, arguments, result) || executeLeftType<Int16>(cond_col, block, arguments, result) || executeLeftType<Int32>(cond_col, block, arguments, result) || executeLeftType<Int64>(cond_col, block, arguments, result) || executeLeftType<Float32>(cond_col, block, arguments, result) || executeLeftType<Float64>(cond_col, block, arguments, result) || executeString(cond_col, block, arguments, result) || executeGenericArray(cond_col, block, arguments, result) || executeTuple(block, arguments, result))) throw Exception("Illegal columns " + arg_then.column->getName() + " and " + arg_else.column->getName() + " of second (then) and third (else) arguments of function " + getName(), ErrorCodes::ILLEGAL_COLUMN); }
3aicheg
27.03.2018 07:09Шутка с «не переходите к описанию проблемы», всё же, не очень хороша — кто-нибудь с хорошим знанием синтаксиса С++ может и спятить, как географ, не нашедший на карте Берингова пролива.
nexmean
27.03.2018 08:04И после этого кто-то говорит о том, что Rust не нужен… 8 из 10 таких ошибок в Rust просто не скомпилируются.
Antervis
27.03.2018 08:588 из 10 приведенных участков написаны на диалекте с++, который старше раста
nexmean
27.03.2018 10:00То есть в современном C++ компиляция данного кода упадёт с ошибкой? Или вы имеете ввиду "старость надо уважать"?
domix32
27.03.2018 11:10Может и не собраться, да. Но суть скорее в размере кодовой базы которую нельзя просто взять и RIIRнуть
Antervis
27.03.2018 12:25сегодня на с++ так уже не пишут, а когда писали — не было раста. Код на современном с++ в свою очередь менее подвержен ошибкам.
Taras-proger
27.03.2018 08:34Ошибки бывают любые и где угодно. Примеры в этой статье — именно это и иллюстрируют. Но некоторые ошибки даже и не сообразишь, как исправить в чужом коде. Например, что значит
(lParam >> 16) && 0xff
? А иногда по внешнему виду нельзя даже понять, ошибка ли это. Например, что такое if (i=5)? Имеется ввиду if (i==5)? Или проверка успешности присваивания хитрым перегруженным оператором-членом, причём, i — нифига не переменная встроенного числового типа, а объект, поддерживает запрет присваивания на основе внутреннего состояния и при срабатывании запрета специальный член, в который заглядывает оператор приведения к bool, получает такое значение, что объект приводится не к true, а к false? Поэтому главное правило программиста гласит: если ты не можешь сказать чего то на языке общения, то не можешь и написать этого на языке программирования. А самая шедевральная ошибка ИМХО — точка с запятой после условия, например: if a>=2; then b:=ln(a);. Она на столько шедевральна, что даже трупопаскаль правильно укажет место, в котором она допущена, а не следующую строку, как это обычно происходит, и не строку в паре десятков экранов от ошибки, как это происходит несколько реже. Причём, говорить студентам, где точка с запятой не ставится, бесполезно. Всё равно сначала поставят, а уберут только в ходе отладки.
Taras-proger
27.03.2018 08:46Ошибки бывают любые и где угодно. Примеры в этой статье — именно это и иллюстрируют. Но некоторые ошибки даже и не сообразишь, как исправить в чужом коде. Например, что значит
(lParam >> 16) && 0xff
? А иногда по внешнему виду нельзя даже понять, ошибка ли это. Например, что такое if (i=5)? Имеется ввиду if (i==5)? Или проверка успешности присваивания хитрым перегруженным оператором-членом, причём, i — нифига не переменная встроенного числового типа, а объект, поддерживает запрет присваивания на основе внутреннего состояния и при срабатывании запрета специальный член, в который заглядывает оператор приведения к bool, получает такое значение, что объект приводится не к true, а к false? Поэтому главное правило программиста гласит: если ты не можешь сказать чего то на языке общения, то не можешь и написать этого на языке программирования. А самая шедевральная ошибка ИМХО — точка с запятой после условия, например: if a>=2; then b:=ln(a);. Она на столько шедевральна, что даже трупопаскаль правильно укажет место, в котором она допущена, а не следующую строку, как это обычно происходит, и не строку в паре десятков экранов от ошибки, как это происходит несколько реже. Причём, говорить студентам, где точка с запятой не ставится, бесполезно. Всё равно сначала поставят, а уберут только в ходе отладки. Единственные места, где я не видел лишних точек с запятой — это: между const и именем первой в разделе константы, между var и именем первой в разделе переменной, между именем и типом переменной или типизированной константы, между type и именем первого в разделе типа и внутри заголовка цикла в паскаль-программе. Но некоторые умудряются писать:
program p1;
type ta=array[1..10] of real;
var a:ta;
a1:real;
a2:real;
a3:real;
a4:real;
a5:real;
a6:real;
a7:real;
a8:real;
a9:real;
a10:real;
i:integer;
begin
a1:=...;
a2:=...;
a3:=...;
a4:=...;
a5:=...;
a6:=...;
a7:=...;
a8:=...;
a9:=...;
a10:=...;
for i:=1 to 10 do
begin
writeln(a[i]);
end;
end.
viktprog
27.03.2018 11:23Не согласен по поводу 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.
Так что это баг автоматического форматирования, а не программиста.
AKudinov
27.03.2018 12:36Наверное, я отстал от моды, но как такое возможно?
sprintf — это макрос, который раскрывается в (!) std::printf!
Вот пишут, что, как и раньше,
int sprintf ( char * str, const char * format,… );
Иначе весь «legacy»-код можно выбросить на помойку. Разве нет?Am0ralist
27.03.2018 14:59+1Вы же понимаете, что делает эта команда?
#define sprintf std::printf
AKudinov
27.03.2018 15:12-1Разумеется.
Но кто, где и зачем такое написал, мне непонятно?
Или это где-то в том же исходнике было, но автор умолчал?
Что-то в стиле
#define TRUE FALSE
foto_shooter Автор
27.03.2018 17:18+1Это define из исходников.
Мне показалось это очевидным, поэтому я не стал писать этот факт прямым текстом.
AKudinov
27.03.2018 13:39Подозреваю, что в примере с mlx5_core_create_qp() просто опечатка при вызове mlx5_cmd_exec(): если предпоследний аргумент &dout, всё встаёт на свои места (и sizeof(dout)). Иначе зачем dout, вообще, нужен?
Приведённого куска кода явно недостаточно, чтобы сделать правильный вывод…
Deranged
27.03.2018 19:17mlx5_core_create_qp()…
т.к. подобная оптимизация не влияет на поведение программы с точки зрения C/C++
Да ну, не может быть. Откуда компилятору знать, что memset не имеет побочных эффектов? Это можно понять только в рантайме. А если memset'у скормить адрес переменной из стека, а размер указать побольше, что бы записалась и следующая? А её уже использовать в коде? Понятно, что это страшный костыль, и того кто так делает к клавиатуре вообще подпускать нельзя, но тем не менее, так сделать можно. И компилятор не имеет права предполагать отстутствие побочных эффектов.Gumanoid
28.03.2018 02:39+1Это будет undefined behavior.
Deranged
28.03.2018 11:34Ну behavior, к сожалению, вполне себе defined, если известна целевая архитектура, а компилятору она известна. Иначе не работали бы так любимые всеми атаки на переполнение стекового буфера. Но не суть. Даже если и undefined — это все равно побочный эффект. А раз вызов функции может иметь побочные эффекты — удалять вызов нельзя. Компилятор может определить отсутствие побочных эффектов по синтаксическому дереву функции. Очевидно, что если, например, это const функция, внутри которой нет ничего подозрительного, типа обращения к mutable членам, или вызова внешних функций, то вызов можно вырезать. Но в случае memset, очевидно, присутствуют команды записи данных по адресу. Что бы понять, что такую запись можно удалить, компилятор по сути должен понять, что не важно, была сделана запись, или нет, программа дальше будет работать одинаково. Т.е. надо понимать, что после записи, этот адрес обязательно будет еще раз перезаписан, до попытки повторно из него прочитать. А еще есть такие вещи, как отображаемые в память файлы, порты ввода вывода. Т.е. если есть только код, который в память только пишет, но нет кода, который из неё обратно читает, это еще не значит, что запись можно не делать. Задача определения отсутствия побочных эффектов при наличии операций записи по адресу — очень крутая, в complie time её решить крайне трудно.
Gumanoid
28.03.2018 12:16Это не так работает. Компилятор делает оптимизации исходя из того, что код написан правильно и UB не может возникнуть ни при каких входных данных.
То есть если написан memset локальной переменной, которая потом не читается, а размер приходит извне, то такой memset можно удалить.
monah_tuk
28.03.2018 05:50Пардон, но:
#define sprintf std::printf
это в чьём коде? Без этих данных решить задачу нет возможности. Тем более, что в Gcc актуальных версий такого определения в cstdio нет, там сделано так:
#undef sprintf namespace std { using ::sprintf; }
Antervis
28.03.2018 06:43видимо, этот дефайн был добавлен для отладки и его забыли убрать
Andrey2008
28.03.2018 08:42Зачем фантазировать… В статье же есть отсылка к первоисточнику: "Любите статический анализ кода". Там показано, что это просто такой дурацкий стиль именования функций, с помощью буквы 's'. Примеры: #define sfstream std::fstream. Злополучный заголовочный файл: definesTypes.h.
monah_tuk
28.03.2018 10:23Просто если лезть в статью, то как быть с пожеланием:
Ни за что не переходите к описанию проблемы, пока не найдёте ошибку в приведённом фрагменте кода.
;-)
Am0ralist
28.03.2018 10:28+2Правильное пожелание, для демонстрации того, что ошибка может быть в другом месте, а конкретное место будет казаться правильным на 100%…
Они ж демонстрируют возможности анализатора.
datacompboy
М--даааа… Это что, кто-то #define true false забабахал уходя?!
Как, ну КААК иначе такие дефайны появляются, блин?
vanxant
Напомнило недавнее из javascript
Здесь после if неразрывный пробел , который будет считаться частью идентификатора, плюс любовь js к автоматической расстановке точек с запятой, что позволяет этому работать.
Конечно, не так злобно, как в примере в статье, зато если хочется нагадить — можно применять избирательно, только по особо сложным местам.