Кажется, что ошибки с забытой точкой запятой весьма распространены. Они часто упоминаются в разных статьях. Но, на самом деле, они встречаются очень редко. Более чем за 10 лет проверки открытых проектов мы нашли всего 3 таких ошибки. Думаю, сказывается, что все знают про эти ошибки и внимательны к ним. Плюс анализаторы и компиляторы, видимо, тоже хорошо помогают обнаруживать их.
Если смотреть не из мира розовых единорогов, где весь код короткий и при этом правильный, то это плохой совет. На мой взгляд, длинный код лучше читается и более защищает от ошибок. По крайней мере в контексте сравнения с nullptr.
Вот, например, как узнать что хотели проверить? Указатель или значение по указателю?
int *p = foo();
if (p) .... // невозможно сказать, верно написано или нет
if (*p) .... // невозможно сказать, верно написано или нет
if (p != nullptr) .... // скорее всего всё хорошо, хотели проверить именно указатель
if (*p != nullptr) .... // ошибка компиляции
Или вот такой реальный пример из проекта Mozilla Firefox:
const char *token = str.get();
if (token == '\0') {
return NS_ERROR_DOM_SYNTAX_ERR; // nothing between commas
}
Если-бы человек был фанатом сокращения текста, он бы написал:
if (!token)
И тогда анализатор PVS-Studio не смог-бы найти ошибку. А за счёт дополнительной информации, что сравнивают не с абстрактным нулём, а с нулевым литералом '\0', анализатор способен предположить, что здесь опечатка:
V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *token == '\0'. svgnumberlist.cpp 96
В общем, забыли разыменовать указатель. Правильно:
Что вы хотите от синтетического примера? :) Его смысл показать, что предупреждение показалось ложным, а оно не ложное.
Естественно, в реальном приложении, всё это где-то глубоко в потрохах. И утечка памяти самая настоящая. Естественно, реальная программа не заканчивает сразу после этого работу, как в моём примере :).
Нет. Такой простой случай сложно придумать :) Правильнее будет сказать, я его "услышал". Хотя можно без кавычек. Это в прямом смысле.
Слышу вдруг коллега в соседней секции: "Ааааа!" и прочие эмоции. Заинтригован, подхожу. Что такое? Он рассказывает и показывает. Я понимаю, что это стоит маленькой заметки :)
Сообщение говорит, что в оператор delete всегда передаётся нулевой указатель. Если бы он не всегда был нулевым, то и предупреждения не будет. Это ведь нормально. А вот передача явного nullptr говорит и какой-то ошибке или избыточном коде. Возможно, опечатались и не ту переменную используют. Не очень понятно, что именно сбивает с толку. на мой взгляд вполне понятное предупреждение. Хотя, конечно, у меня может быть взгляд замылен :).
Интерес к C# и Java растёт, но медленно. Думаю, мы сами в этом виноваты, так как команде не хватает сил более активно заниматься их продвижением. Про Java, например, вообще давно статей не писали :(. На тему C++ у нас куда больше разнообразного материала и активностей. В том числе больших и сильных, таких как мини-книга про вредные C++ советы.
MISRA - интерес есть, но у малого количества пользователей. Хорошо, что сделали, но существенно на жизнь это не повлияло. По запросам пользователей скоро будем дорабатывать это направление и делать ещё некоторые диагностики.
C++ остаётся основным направлением? - Да. Оно самое старое, поэтому там больше всего клиентов и поддержки. И с продвижением у нас в этом ловчее получается.
На это полагаться нельзя и даже вредно про это думать. Неизвестно как и где будет выполняться программа. Более того, код может быть заимствован для других проектов, работающих в других условиях. Всегда следует исходить из того, что new может сгенерировать исключение.
P.S. Раньше подобное приходилось про malloc писать, теперь new.... эх...
В данном случае согласен. В более общем, указатель может инициализироваться при одних условиях и оставаться нулевым в других. Программисты знают про это и пишут лишние проверки перед delete (и перед free в C). Я имел в иду, что такие проверки не нужны, так как delete (и free) корректно обработают нулевой указатель.
Да, еще есть new(std::nothrow), но он тут ни при чём.
Просто интересно, а почему не упоминается статический анализ кода? По идее он идёт ещё раньше или параллельно с юнит-тестированием. Или статический анализ относится уже не к команде тестирования? Хотя юнит-тесты, вроде тоже не задача тестировщиков.
Мы так никому не отвечаем. Более того, у нас есть несколько вариантов бесплатного лицензирования для студентов, открытых проектов и т.д. Вы или спутали PVS-Studio с другим анализатором или произошло ещё какое-то недоразумение/непонимание.
Такое вполне обосновано. Оно служит для непонятных низкоуровневых действий. Я про это упоминаю (про сложные конструкции). Фактически это более высокоуровневое описание этих действий.
За пример с va_arg спасибо. Выписал в список идей для новых диагностик.
По поводу массивов единичного размера в конце структур. Да, это особая тема, про которую я забыл упомянуть. Кстати, этот момент не очень понятно, как описать. Ибо всё будет работать, так как компиляторы точно в курсе такого приёма создания структур переменного размера. Другое дело, я не уверен, как всё это смотрится с теоретической точки зрения.... :)
Это было интересно. Обычно, я что-то такое пишу. А тут довелось самому поиграть в угадайку. Спасибо.
Кстати, пример с
int a = index + pockets[++index];
напомнил мне про наш один вопрос - Глубина кроличьей норы или собеседование по C++ в компании PVS-Studio :)P.S. А кому интересно узнать про глубину глубин с Unicode, приглашаю сюда - Атака Trojan Source для внедрения в код изменений, незаметных для разработчика.
Ложные представления программистов о неопределённом поведении.
Спасибо за статью. По духу близко к моей подборке вредных советов :)
Пара мыслей про описанные баги.
Кажется, что ошибки с забытой точкой запятой весьма распространены. Они часто упоминаются в разных статьях. Но, на самом деле, они встречаются очень редко. Более чем за 10 лет проверки открытых проектов мы нашли всего 3 таких ошибки. Думаю, сказывается, что все знают про эти ошибки и внимательны к ним. Плюс анализаторы и компиляторы, видимо, тоже хорошо помогают обнаруживать их.
И наоборот, ошибка с исчезновением
memset
кажется чем-то экзотическим и редким. Но это очень распространённая потенциальная уязвимость. Мы их обнаружили 100500 штук и продолжает обнаруживать. Хотя, казалось бы на эту тему тоже уже много написано: CWE-14, Zero and forget -- caveats of zeroing memory in C, Безопасная очистка приватных данных.Сейчас у нас нет планов по реализации поддержки Golang.
Если смотреть не из мира розовых единорогов, где весь код короткий и при этом правильный, то это плохой совет. На мой взгляд, длинный код лучше читается и более защищает от ошибок. По крайней мере в контексте сравнения с
nullptr
.Вот, например, как узнать что хотели проверить? Указатель или значение по указателю?
Или вот такой реальный пример из проекта Mozilla Firefox:
Если-бы человек был фанатом сокращения текста, он бы написал:
И тогда анализатор PVS-Studio не смог-бы найти ошибку. А за счёт дополнительной информации, что сравнивают не с абстрактным нулём, а с нулевым литералом '\0', анализатор способен предположить, что здесь опечатка:
V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *token == '\0'. svgnumberlist.cpp 96
В общем, забыли разыменовать указатель. Правильно:
Что вы хотите от синтетического примера? :) Его смысл показать, что предупреждение показалось ложным, а оно не ложное.
Естественно, в реальном приложении, всё это где-то глубоко в потрохах. И утечка памяти самая настоящая. Естественно, реальная программа не заканчивает сразу после этого работу, как в моём примере :).
Нет. Такой простой случай сложно придумать :) Правильнее будет сказать, я его "услышал". Хотя можно без кавычек. Это в прямом смысле.
Слышу вдруг коллега в соседней секции: "Ааааа!" и прочие эмоции. Заинтригован, подхожу. Что такое? Он рассказывает и показывает. Я понимаю, что это стоит маленькой заметки :)
Про V575.
Сообщение говорит, что в оператор
delete
всегда передаётся нулевой указатель. Если бы он не всегда был нулевым, то и предупреждения не будет. Это ведь нормально. А вот передача явногоnullptr
говорит и какой-то ошибке или избыточном коде. Возможно, опечатались и не ту переменную используют. Не очень понятно, что именно сбивает с толку. на мой взгляд вполне понятное предупреждение. Хотя, конечно, у меня может быть взгляд замылен :).Интерес к C# и Java растёт, но медленно. Думаю, мы сами в этом виноваты, так как команде не хватает сил более активно заниматься их продвижением. Про Java, например, вообще давно статей не писали :(. На тему C++ у нас куда больше разнообразного материала и активностей. В том числе больших и сильных, таких как мини-книга про вредные C++ советы.
MISRA - интерес есть, но у малого количества пользователей. Хорошо, что сделали, но существенно на жизнь это не повлияло. По запросам пользователей скоро будем дорабатывать это направление и делать ещё некоторые диагностики.
CVE - как я понимаю, имеется в виду SCA (software composition analysis). Пока не понятно.
C++ остаётся основным направлением? - Да. Оно самое старое, поэтому там больше всего клиентов и поддержки. И с продвижением у нас в этом ловчее получается.
На это полагаться нельзя и даже вредно про это думать. Неизвестно как и где будет выполняться программа. Более того, код может быть заимствован для других проектов, работающих в других условиях. Всегда следует исходить из того, что
new
может сгенерировать исключение.P.S. Раньше подобное приходилось про malloc писать, теперь new.... эх...
В данном случае согласен. В более общем, указатель может инициализироваться при одних условиях и оставаться нулевым в других. Программисты знают про это и пишут лишние проверки перед
delete
(и передfree
в C). Я имел в иду, что такие проверки не нужны, так какdelete
(иfree
) корректно обработают нулевой указатель.Да, еще есть
new(std::nothrow)
, но он тут ни при чём.Просто интересно, а почему не упоминается статический анализ кода? По идее он идёт ещё раньше или параллельно с юнит-тестированием. Или статический анализ относится уже не к команде тестирования? Хотя юнит-тесты, вроде тоже не задача тестировщиков.
Спасибо. Поправил.
Мы так никому не отвечаем. Более того, у нас есть несколько вариантов бесплатного лицензирования для студентов, открытых проектов и т.д. Вы или спутали PVS-Studio с другим анализатором или произошло ещё какое-то недоразумение/непонимание.
PDF: https://www.slideshare.net/Andrey_Karpov/60-terrible-tips-for-a-c-developer
Такое вполне обосновано. Оно служит для непонятных низкоуровневых действий. Я про это упоминаю (про сложные конструкции). Фактически это более высокоуровневое описание этих действий.
Написал обыкновенный вариант - молодец.
Дополнительно сказал, что был какой-то прём/инструкция - молодец++.
За пример с
va_arg
спасибо. Выписал в список идей для новых диагностик.По поводу массивов единичного размера в конце структур. Да, это особая тема, про которую я забыл упомянуть. Кстати, этот момент не очень понятно, как описать. Ибо всё будет работать, так как компиляторы точно в курсе такого приёма создания структур переменного размера. Другое дело, я не уверен, как всё это смотрится с теоретической точки зрения.... :)
Спасибо, подумаем.
PDF