Комментарии 65
"Обратите внимание на выделенный фрагмент. Для форматирования отступов используется символ табуляции и под условие сдвинули два оператора."
Когда я участвовал в проекте, связанным с поддержкой своей версии ядра FreeBSD, руководство требовало соблюдения его codingstyle, запрещающего фигурные скобки вокруг единственного оператора. И такая ошибка была очень частой, не только у меня, но и у значительно более опытных разработчиков. Я пытался бороться, но мои коммиты за это заворачивали на code review.
Когда я участвовал в проекте, связанным с поддержкой своей версии ядра FreeBSD, руководство требовало соблюдения его codingstyle, запрещающего фигурные скобки вокруг единственного оператора. И такая ошибка была очень частой, не только у меня, но и у значительно более опытных разработчиков. Я пытался бороться, но мои коммиты за это заворачивали на code review.
Имхо, плохая практика. Всегда при составлении codingstyle настаиваю на обязательном включении в фигурные скобки единственного оператора.
А я не вижу смысл писать 3 строки (или даже 4, если кодегайд требует переноса открывающей скобки на новую строку), вместо 2. Просто следить нужно за тем, что пишешь… Тут вон недавно был пост, в котором и вовсе break говорили не использовать, а вызов безымянной функции =\ Меня такие вещи возмущают — почему если кто-то неаакуратен, то ограничивать себя должна вся команда?
Проблема в том, что не все следят. Я пришёл к выводу о необходимости такого правила, когда обноружил баг в модуле, который на прошлом тестировании работал, вызванный вот такими строками:
if (zoom == 1)
// log(items);
self.redraw();
А вся команда должна себя ограничивать потому, во-первых, любой может быть невнимателен, а во-вторых, потом вся команда будет искать этот баг
if (zoom == 1)
// log(items);
self.redraw();
А вся команда должна себя ограничивать потому, во-первых, любой может быть невнимателен, а во-вторых, потом вся команда будет искать этот баг
Радует, что GCC6 будет ловить (-Wmisleading-indentation) хотя бы часть подобных косяков.
Да, полезно. Кстати в PVS-Studio тоже давно есть подобная диагностика V628. Анализаторы всегда на шаг впереди компиляторов в этом плане. :)
Потому что две секунды экономии времени одного невнимательного выливаются двумя днями потерянного времени всей команды на отлов взявшегося "из ниоткуда" загадочного бага.
"Уставы пишутся кровью." Требовать, чтобы это была именно ваша кровь, достаточно неумно.
"Уставы пишутся кровью." Требовать, чтобы это была именно ваша кровь, достаточно неумно.
Может быть, лучше использовать анализатор, подобный PVS, чем писать странные конструкции?
Нет, я согласен, это хорошая практика, чтобы не наделать ошибок на "ровном месте", "без ничего". Если мы текст набираем в блокноте, собираем из командной строки. Такие конструкции заметны и в распечатке. Но если есть возможность и не писать лишнего, и проверить на ошибки — почему бы и нет?
Нет, я согласен, это хорошая практика, чтобы не наделать ошибок на "ровном месте", "без ничего". Если мы текст набираем в блокноте, собираем из командной строки. Такие конструкции заметны и в распечатке. Но если есть возможность и не писать лишнего, и проверить на ошибки — почему бы и нет?
Как говорится, "чисто не там, где подметают, а там, где не сорят" ©
А вообще:
А вообще:
- Анализаторы не для всех языков есть
- Анализатор стоит денежков, а начальство зачастую не настолько умное, чтобы понимать, "какая от этого
котаединорога польза."
Как говорится, «чисто не там, где подметают, а там, где не сорят»
Слышал наоборот: чисто не там, где не сорят, а там, где убирают. Эту фразу крутят, как хотят.
А вообще:
Анализаторы не для всех языков есть
Анализатор стоит денежков, а начальство зачастую не настолько умное, чтобы понимать, «какая от этого кота единорога польза.»
Ну, мы же про С++ конкретно. Стоит, да, но такой подход тоже имеет право на жизнь — нормальный код и анализатор.
Слышал наоборот: чисто не там, где не сорят, а там, где убирают.Ну вообще-то миф о самозарождении
Ну, мы же про С++ конкретно.Не знаю насчёт «мы», лично я нигде слова «C++» не упомянал. Я, скажем, работаю c Ruby, но там точно так же — один неаккуратный чудак может испортить неделю всей команде.
Стоит, да, но такой подход тоже имеет право на жизнь — нормальный код и анализатор.Конечно. Они ни в коей мере не являются взаимоисключающими.
Ну вообще-то миф о самозарождении мусора мышей в грязном белье был развенчан уже давно
Никто не говорит о "самозарождении мусора". Речь о том, что чисто будет в обеих ситуациях — и если не сорят (ваше), и если сорят, но убирают (мое высказывание).
Следовательно, моя гипотеза верна, потому что по Вашей (количество подметающих = 0) на Луне должна быть нехилая куча мусора.
Нет, это софистика: когда речь идет об уборке ("… там, где убирают"), подразумевается, что мусор есть до начала уборки — иначе убирать не надо. Ваше "количество подметающих=0" неверно, должно быть "количество подметающих имеющийся мусор=0". А так как на Луне нет субъектов, нет мусора, то это некорректный пример.
Не знаю насчёт «мы», лично я нигде слова «C++» не упомянал
Вся статься — об анализаторе для С++ ©. Ветка, где мы общаемся, начинается с реплики по поводу куска статьи, который говорит об ошибке, связанной с coding style в С проекте.
Вернуться на хабр без кармы из-за одного комментария все же больнее, чем найти место без открывающейся скобки =\
Просто. Следить. Ага. "Да и вообще, зачем вам тестеры? Вы же профессионалы и не делаете ошибок в коде?"
И я. Хоть меня и не всегда слушают, не умею я людей убеждать.
Радует, что в Rust скобки обязательны, хочется попробовать его в деле...
Радует, что в Rust скобки обязательны, хочется попробовать его в деле...
Swift от Apple с Вами согласен. Там отсутствие скобок — ошибка...
Любопытно, что в OpenJDK в разных пакетах разный стиль. В java.time, например, к моим патчам требовали фигурные скобки, а в java.util.stream куча кода, где их не ставят. Видимо, общего требования нет.
В приведенных выше отрывках кода встречаются однооператорные конструкции, заключенные в фигурные скобки. Так что, видимо, не такая уж это и догма.
Я не спец в плюсах, но какой смысл в коде задержки:
for (i=0; i < 10; i++) {
for (i = 0; i < 10000; i++);
}
пустой цикл разве компилятор не выпилит нафиг?
for (i=0; i < 10; i++) {
for (i = 0; i < 10000; i++);
}
пустой цикл разве компилятор не выпилит нафиг?
Сверху ставят штамп: "собирать только с выключенной оптимизацией" и стараются не дышать ;)
ЗЫ еще пишут volatile int i, но в общем тоже практика не фонтан...
ЗЫ еще пишут volatile int i, но в общем тоже практика не фонтан...
Это не совсем пустой цикл, в нём изменяется внешняя переменная 'i'.
При оптимизации должно выпилиться в i=10;
Правда в 10?
Совсем не 10)))
Не правда. 10001 похоже. Невнимательность...
Я вам по секрету скажу, что i будет равно 10000 после чего выйдет из обоих циклов. Тем более, в статье об этом явно написано.
i будет 10001. В статье явно написано, что тут не 10*10000 итераций, а в 10 раз меньше. Т.е. я считал именно итерации цикла, их 10000.
Точно, до выхода из первого цикла будет еще увеличение на 1.
Мне кажется, что ошибка в миллиарды долларов, это не нулевая ссылка, а цикл for. Сложнее представить вещь, в которой сложнее не ошибиться по невнимательности.
Без цикла
Сам сотню раз так ошибался в таких языках.
for его роль занял бы цикл while — а там ошибиться еще проще. К примеру, вовсе забыть об изменении счетчика цикла.Сам сотню раз так ошибался в таких языках.
Поэтому и придумали диапазоны, map, reduce и т.д.
Вот только для их адекватного использования нужны замыкания, а для замыканий — либо шаблоны, либо объекты с наследованием, полиморфизмом и виртуальными функциями. Ну или функциональный язык программирования.
Слишком сложная замена для простого цикла. Для такого простого языка как Си без плюсов избавиться от цикла for — невозможно.
Слишком сложная замена для простого цикла. Для такого простого языка как Си без плюсов избавиться от цикла for — невозможно.
По крайней мере не я один ошибся)
int retry = 0, i = 0;
int HostDiag;
....
for (i = 0; i < 10000; i++) ;
HostDiag = (uint32_t)MFI_READ4(sc, MFI_HDR);
while (!( HostDiag & DIAG_WRITE_ENABLE)) {
for (i = 0; i < 1000; i++);Выше по коду ещё несколько таких мест, поэтому я и сделал акцент, что 2 вложенных цикла в результате дадут в 10 раз меньше итераций, чем вроде бы выглядит.
Писать Typo's неправильно. Правильно писать Typos. Апостроф не нужен. Сорри, но режет глаз.
Что-то дофига ошибок для freebsd...
Микрофикс:
bzero(dst, sizeof(*dst));
Зная, что PVS найдет дофига багов, уже не столько интересно о них читать, сколько — как отреагировали на них разработчики? Вам вообще фидбек приходит?
Разработчики сначала вежливо поговорили, потом кто-то попытался установить ту версию, которая собственно проверялась. По его подсчетам, получилась версия пятилетней давности. Потом его поправили, и выяснилось что версия была все-таки свежая.
https://lists.freebsd.org/pipermail/freebsd-doc/2016-February/026369.html
https://lists.freebsd.org/pipermail/freebsd-doc/2016-February/026369.html
Я сразу упоминал, что использовал Git репозиторий, но кто-то всё равно попытался найти этот номер ревизии в Subversion. На данный момент уже уточнили этот момент.
Я так понимаю, Subversion у них — основной репозиторий, ибо так сложилось исторически. А на гитхабе — копия для широких масс, притом неофициальная (ее даже на оф. сайте нет). Так что определение номера ревизии из SVN — это и правда первое что следовало сделать.
Но вот способ, которым это попытались сделать сначала (подсчет числа коммитов в гите исходя из предположения что каждой ревизии соответствует ровно один коммит)...
Но вот способ, которым это попытались сделать сначала (подсчет числа коммитов в гите исходя из предположения что каждой ревизии соответствует ровно один коммит)...
В этом случае я сразу написал разработчикам ещё до написания статьи и выслал им отчёт анализатора. Нас поблагодарили за помощь. Один разработчик прокомментировал несколько ложных срабатываний с указателями. Объяснив, что на самом деле там не очень тривиальный код и всё будет работать правильно.
Несомненно, PVS молодцы, учитывая объем проекта. Но я бы хотел обратить внимание, как круто ребята рисуют/подбирают картинки к статье! После чтения кода, очень разгружает картинку в голове. Спасибо за интересную статью!
Кому нравятся картинки — напоминаем, что с сайта можно бесплатно скачать кружки, майки, обои с единорогами.
Это в том смысле, что с кода блевать тянет? :)
safe.c 1596 — не бесконечный цикл, а просто лишняя проверка.
Там внутри цикла правильные проверки есть для выхода.
Там внутри цикла правильные проверки есть для выхода.
Было бы здорово если добавили бы поддержку компилятора Texas Instruments (http://processors.wiki.ti.com/index.php/TI_Compiler_Information). Это возможно?
Спроса нет :-(, к сожалению. Редкий слишком зверь.
А почему вообще нужна привязка к компилятору? По идее же проводится анализ исходных кодов, проблема как я понимаю только слинковать.
НЛО прилетело и опубликовало эту надпись здесь
It is possible that macro expansion resulted in incorrect evaluation order. Check expression: chan — 1 * 20.Интересно, а в этом случае PVS-Studio как-то сравнивает код до раскрытия макроса и код после? В смысле, она знает, что тут изначально был макрос? Или просто по внешнему виду выражения после препроцессинга догадалась?
Кстати, большие дискуссии развернулись на сайте linux.org.ru и slashdot.org. Правда конструктивного в них не очень много. Тем не менее, возможно кому-то будет интересно почитать:
Эпичное продолжение: Как найти 56 потенциальных уязвимостей в коде FreeBSD за один вечер.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
PVS-Studio покопался в ядре FreeBSD