Существует две возможности: либо мы одиноки во Вселенной, либо нет. Обе одинаково ужасны. (с) Артур Чарльз Кларк
Дискуссии о том, одиноки ли мы во Вселенной, будоражат умы людей уже не один десяток лет. Серьёзное отношение к этому вопросу имеет проект SETI, занимающийся поиском внеземных цивилизаций и возможностью вступления с ними в контакт. Об анализе исходного кода одного из проектов этой программы - SETI@home - и пойдёт речь в данной статье.
Подробнее о проекте
SETI@home - научный некоммерческий проект добровольных вычислений, использующий свободные ресурсы на компьютерах добровольцев для поиска радиосигналов внеземных цивилизаций. Проект основан на открытой программной платформе для организации распределённых вычислений - BOINC, написанной на C++.
В качестве инструмента анализа использовался статический анализатор C/C++ кода - PVS-Studio. Исходные коды проекта SETI@home доступны на официальном сайте. Инструкцию о том, как собирать проект, найти также не сложно, так что, собрав всё необходимое и приготовив чашечку кофе, я приступил к делу.
Результаты проверки
Признаюсь, что перед анализом проекта я был в предвкушении того, сколько проблемных мест удастся обнаружить. Но на моё удивление действительно интересных фрагментов кода (проблемных) оказалось не так уж много, что говорит о его качестве.
Тем не менее, подозрительные места были, и некоторые из них я бы хотел рассмотреть.
Для разогрева
Примеры кода в этом разделе нельзя подвести под какую-то одну категорию, как например "указатели" или "циклы", так они имеют разную тематику, но по-своему интересны.
Предупреждение анализатора: V560 A part of conditional expression is always true: 0xff. seti_header.cpp 96
Подозрительным выглядит оператор '&&', который используется для получения целочисленного значения. Возможно, в данном случае был необходим оператор '&', так как иначе переменная 'splitter_version' всегда будет принимать одно из двух значений: 0 или 1.
Конечно, вероятность того, что подразумевалась прибавка к 'splitter_version' 0 или 1 есть, но, думаю, вы согласитесь, что она не очень велика, и в таком случае можно было использовать более понятный код (например, тернарный оператор).
Следующий подозрительный фрагмент кода - методы, которые должны возвращать значение, но, тем не менее, ничего не возвращают. Более того - они имеют пустые тела. Такой код как минимум выглядит подозрительно. Предлагаю взглянуть самим:
V591 Non-void function should return a value. x86_float4.h 237
V591 Non-void function should return a value. x86_float4.h 239
V591 Non-void function should return a value. x86_float4.h 241
Как видно из этого фрагмента, ни один из методов ничего не возвращает. Я специально выписал отдельно данный участок кода, и был несколько удивлён, увидев, что компиляция проходит успешно. Никаких предупреждений компилятора также не было. Но только до тех пор, пока данные методы не будут вызваны. При попытке сделать это всё же возникает ошибка компиляции.
Что это было: задел на будущее или ошибка - сказать сложно, так как никаких комментариев к данному коду не было. Просто имейте ввиду то, что я написал выше.
Но не будем зацикливаться на этом примере, лучше взглянем, что ещё удалось найти.
Предупреждение анализатора: V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. xml_util.h 891
Как понятно из кода, в ходе парсинга входных данных необходимо было получить значение длины (переменная 'length').
Что подразумевалось? В строке осуществляется поиск подстроки "length=", если она обнаружена, индекс начала подстроки записывается в переменную 'len'. После этого исходная строка преобразуется в C-строку, из которой при помощи оператора индексации извлекается необходимое значение длины. В качестве вычисления индекса символа, хранящего значение длины, как раз используется индекс подстроки "length=" и её длина.
Однако из-за приоритета операций (или неправильно расставленных скобок в условии, видно, что они дублируются) всё пойдёт не так. Сначала будет выполнено сравнение со значением 'npos', а результат этого сравнения (0 или 1) будет записан в переменную 'len', что приведёт к неправильному вычислению индекса массива.
В ходе просмотра лога анализа я наткнулся на парочку интересных макросов. Предлагаю взглянуть и вам:
Сразу хочу сказать, что этот макрос по ходу кода встречался неоднократно. Непонятно, почему бы просто не генерировать исключение. Вместо этого в коде встречается непонятная лексема и присутствует цикл, для которого выполняется только одна итерация. Подход интересный, но к чему такой велосипед - неясно.
Указатели и работа с памятью
Для разнообразия - пример кода с указателями. Как правило, во фрагментах кода, содержащих работу с указателями или адресами, вероятность наступить на грабли порядком возрастает. Поэтому они вызывают больший интерес.
Предупреждение анализатора: V595 The 'ChirpSteps' pointer was utilized before it was verified against nullptr. Check lines: 138, 166. chirpfft.cpp 138
Анализатор предупреждает о том, что указатель используется до того, как выполняется проверка, на то, является ли он нулевым. Если не удастся выделить память и функция 'calloc' вернёт значение 'NULL', будет выполнено разыменовывание нулевого указателя, что, как все мы прекрасно знаем, не очень хорошо.
Другой момент заключается в том, что функция 'free' вызывается только в том случае, если указатель не равен 'NULL'. Эта проверка избыточна, так как функция 'free' без проблем обрабатывает нулевые указатели.
Другой участок кода с подозрительным использованием функции 'memset'. Давайте посмотрим:
Код
int ReportTripletEvent(....) { .... static int * inv; if (!inv) inv = (int*)calloc_a(swi.analysis_cfg.triplet_pot_length, sizeof(int), MEM_ALIGN); memset(inv, -1, sizeof(inv)); for (i=0;i<swi.analysis_cfg.triplet_pot_length;i++) { j = (i*pot_len)/swi.analysis_cfg.triplet_pot_length; if (inv[j] < 0) inv[j] = i; .... } .... }
Предупреждение анализатора: V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. analyzereport.cpp 271
Из данного фрагмента кода видно, что сначала выделяется память под массив, после чего его элементы заполняются значением '-1', а после с ними происходит работа. Но вот в функцию 'memset' третьим параметром передаётся не размер массива, а размер указателя. Для правильного заполнения массива необходимыми символами третьим аргументом следовало передавать размер буфера.
Циклы
Во многих проектах встречаются циклы, тело которых либо выполняется бесконечно, либо не выполняется вообще. SETI@home не стал исключением. С другой стороны - здесь последствия не выглядят такими критичными, как в некоторых других проектах.
Предупреждение анализатора: V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. schema_master.cpp 9535
Ошибка весьма тривиальна. Как все мы знаем, тело цикла 'for' выполняется, пока его условное выражение истинно. Здесь же уже на первой итерации условие будет ложным, так что сразу будет осуществлён выход из цикла. Лично я не могу понять, что здесь подразумевалось, но тем не менее тело этого цикла никогда не будет выполняться.
Аналогичный фрагмент кода встретился ещё раз, но в другом методе другого класса:
V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. schema_master.cpp 11633
Не столь прозрачный, но потенциально ошибочный пример:
Предупреждение анализатора: V663 Infinite loop is possible. The 'cin.eof()' condition is insufficient to break from the loop. Consider adding the 'cin.fail()' function call to the conditional expression. sqlblob.h 58
Так как мы рассматриваем циклы, несложно догадаться, что ошибка - в условии выхода из цикла 'while'. Хотя многие наверняка не обнаружат ничего подозрительного, так как применяемый здесь метод выглядит вполне стандартным. Но подводный камень есть, иначе этого примера в статье не было бы.
Дело в том, что при возникновении сбоя чтения данных такой проверки будет недостаточно. В таком случае метод 'eof()' будет постоянно возвращать 'false', как следствие - бесконечный цикл.
Для исправления ошибки необходимо добавить дополнительное условие. Тогда цикл будет выглядеть следующим образом:
Код
while(!i.eof() && !i.fail()) { //do something }
Прочие подозрительные места
Аккуратными нужно быть и с битовыми операциями. В ходе анализа встретился участок кода, приводящий к неопределённому поведению:
Код
int seti_analyze (ANALYSIS_STATE& state) { .... int last_chirp_ind = - 1 << 20, chirprateind; .... }
Предупреждение анализатора: V610 Undefined behavior. Check the shift operator '<<'. The left operand '-1' is negative. analyzefuncs.cpp 177
Как видно из кода, переменная инициализируется значением, полученным в результате битового сдвига. И всё бы ничего, но левый операнд отрицателен, а согласно стандарту C++11, эта операция приводит к неопределённому поведению.
Выходит палка о двух концах. С одной стороны - подобный код давно и неоднократно используется, с другой - в стандарте всё же указано, что данный код приводит к неопределённому поведению.
Окончательное решение остаётся за программистом, но обратить внимание на это стоит.
Неоднократно встречался код, где одной и той же переменной дважды присваивались различные значения, причём между этими присваиваниями никаких других операций с переменной не производилось. Один из примеров такого кода:
Код
int checkpoint(BOOLEAN force_checkpoint) { int retval=0, i, l=xml_indent_level; .... retval = (int)state_file.write(str.c_str(), str.size(), 1); // ancillary data retval = state_file.printf( "<bs_score>%f</bs_score>\n" "<bs_bin>%d</bs_bin>\n" "<bs_fft_ind>%d</bs_fft_ind>\n", best_spike->score, best_spike->bin, best_spike->fft_ind); .... }
Предупреждение анализатора: V519 The 'retval' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 450, 452. seti.cpp 452
В данной ситуации тяжело сказать, что подразумевалось или как это необходимо исправить. Но программист, писавший код, возможно, поймёт причину такого использования переменной. Нам же остаётся только удивляться и строить догадки.
Встретились ещё четыре подобных участков кода. Соответствующие сообщения анализатора:
V519 The 'retval' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 470, 472. seti.cpp 472
V519 The 'retval' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 490, 492. seti.cpp 492
V519 The 'retval' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 513, 515. seti.cpp 515
V519 The 'retval' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 533, 536. seti.cpp 536
V519 The 'lReturnValue' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 85, 97. win_util.cpp 97
Возможно, эти переменные использовались просто для просмотра значений, которые возвращают функции при отладке кода. Тогда ничего опасного нет и эти предупреждения можно игнорировать или подавить одним из множества способов, которые предоставляет анализатор PVS-Studio.
Напоследок приведу пример, где несколько нерационально используется функция 'strlen':
Код
int parse_state_file(ANALYSIS_STATE& as) { .... while(fgets(p, sizeof(buf)-(int)strlen(buf), state_file)) { if (xml_match_tag(buf, "</bt_pot_min")) break; p += strlen(p); } .... }
Предупреждение анализатора: V814 Decreased performance. Calls to the 'strlen' function have being made multiple times when a condition for the loop's continuation was calculated. seti.cpp 770
Ввиду того, что буфер (переменная 'buf') не изменяется в ходе выполнения цикла, нет никакой необходимости вычислять его длину на каждой итерации. Возможно, целесообразнее было бы завести для этого отдельную переменную, с которой и производить сравнение. Это не столь заметно при малых размерах буфера, но при больших, когда количество итераций существенно больше, может быть куда заметнее.
Данный код встречался неоднократно, вот ещё несколько подобных сообщений:
V814 Decreased performance. Calls to the 'strlen' function have being made multiple times when a condition for the loop's continuation was calculated. seti.cpp 784
V814 Decreased performance. The 'strlen' function was called multiple times inside the body of a loop. xml_util.cpp 663
V814 Decreased performance. The 'strlen' function was called multiple times inside the body of a loop. xml_util.cpp 686
Что ещё удалось найти?
Были и другие предупреждения анализатора, но фрагменты кода я счёл не настолько интересными, чтобы отдельно выписывать их и разбирать. Просто можете прочесть этот раздел и узнать, что ещё встретилось в ходе проверки.
Например, попадались "висящие" массивы, которые объявляются, но никак не используются. Благо, что размер их был фиксированным и небольшим. Тем не менее, стековая память под них выделялась, что нецелесообразно.
Также несколько раз встречалось разыменовывание указателя с последующим его увеличением (*p++). При этом значение, хранящееся в указателе, никак не использовалось. Из примеров было видно, что подразумевалось просто изменение самого указателя, но всё же его зачем-то разыменовывали. Данный код потенциально ошибочен, так как порой может подразумеваться изменение значения, хранящегося в указателе, а не его самого. Поэтому относиться с недоверием к этим предупреждениям не стоит.
Неоднократно встречалась функция 'fprintf', форматная строка которой не соответствовала передаваемым в функцию фактическим аргументам. Это приводит к неопределённому поведению и может служить причиной, например, вывода на экран бессмыслицы.
Заключение
После проверки у меня осталось двоякое впечатление. С одной стороны - я был несколько расстроен тем, что ошибок в коде нашлось куда меньше, чем ожидалось, следовательно - меньше материала для статьи. С другой - проект всё же был проверен, и это было интересно. А малое количество ошибок свидетельствует о качестве кода, что тоже хорошо.
Что ещё добавить? Устанавливайте клиент SETI@home - помогайте в поиске внеземных цивилизаций, устанавливайте PVS-Studio - он поможет вам в поиске ошибок в исходных кодах, написанных на C/C++.
Спор BlackBerry и Typo окончательно завершился 2015-06-01 19:04 В прошлом году BlackBerry подала в суд на компанию Typo, выпускавшую специальные чехлы с механической клавиатурой под названием Typo Keyboard Case, которые предназначались для смартфонов Apple
Экономика Беларуси потеряла $3 млрд из-за российских проблем 2015-06-01 18:52 Из-за обвала российских рынков, санкций, эмбарго белорусская экономика потеряла почти $3 млрд. Об этом говорится в докладе, который обсудили президент Республики Беларусь Александр Лукашенко...
Криштиану Роналду выиграл рекордную "Золотую бутсу" 2015-06-01 18:30 Португальский нападающий мадидского Реала Криштиану Роналду, забив в чемпионате Испании 48 голов, в четвертый раз за карьеру стал обладателем "Золотой бутсы"...
Под Одессой нашли заблудившихся в катакомбах детей 2015-06-01 17:45 Детей, заблудившихся в субботу в катакомбах под Одессой, нашли. Об этом " Сегодня" рассказали в пресс-службе ГСЧС в Одесской области...
Под Одессой нашли заблудившихся в катакомбах детей 2015-06-01 17:45 Детей, заблудившихся в субботу в катакомбах под Одессой, нашли. Об этом " Сегодня" рассказали в пресс-службе ГСЧС в Одесской области...
Число погибших на Донбассе превысило 6,3 тыс. человек, - ООН 2015-06-01 17:21 Число погибших в результате конфликта на Донбассе составило 6 тыс. 362 человек. Об этом говорится в докладе мониторинговой миссии Управления верховного комиссара ООН...
Google Project Loon: система автозапуска и новая схема передачи веб-трафика 2015-06-01 18:54 Компания Google рассказала о том, как продвигается развитие проекта Loon по формированию флотилии гигантских воздушных шаров для предоставления доступа в Интернет жителям бедных и труднодоступных регионов. Project Loon был организован ещё в 2011 году в стенах секретного подразделения Google X Lab. Суть инициативы заключается в том, что специальные аэростаты, находясь на стратосферных высотах, будут передавать веб-трафик наземным потребителям. Для контроля положения шаров служат воздушные потоки и солнечная энергия. Вся система использует комплексные модели предсказания траекторий с учётом самых разных факторов.
ECS PCoS: компьютер-брелок на платформе Intel Bay Trail 2015-06-01 18:29 Компания ECS (Elitegroup Computer Systems) анонсировала миниатюрный компьютер PCoS, который должен составить конкуренцию Intel Compute Stick. Устройство, выполненное в виде брелока, построено на платформе Intel Bay Trail — производитель выбрал процессор Atom Z3735F. Этот чип располагает четырьмя вычислительными ядрами с номинальной тактовой частотой 1,33 ГГц (динамически повышается до 1,83 ГГц), а также графическим контроллером Intel HD Graphics. Среднее значение рассеиваемой тепловой энергии равно 2 Вт.
Computex 2015: «умные» часы ASUS ZenWatch 2 доступны в 41-мм и 37-мм вариантах 2015-06-01 18:00 На пресс-конференции в рамках выставки Computex 2015 компания ASUS представила «умные» часы нового поколения ZenWatch 2 под управлением Android Wear. Новинка отличается от предыдущей версии, получившей неплохие отклики пользователей, усовершенствованным дизайном и слегка улучшенной функциональностью. ZenWatch 2 будет выпускаться в двух версиях, различающихся размерами. Модель ZenWatch 2 (WI501Q) получила 41-мм корпус и ремешок шириной 22 мм, а у ZenWatch 2 (WI502Q) габариты немного меньше: 37-мм корпус и 18-мм ремешок. Насколько отразилось изменение габаритов у младшей модели на размерах диагонали AMOLED-дисплея, защищённого от повреждений изогнутым стеклом 2,5D Gorilla Glass 3, пока не известно.
