Записи Друзья Комментарии
rss rss hh new
Аватар rss_rss_hh_new

 -Поиск по дневнику

Поиск сообщений в rss_rss_hh_new

 -Подписка по e-mail

 

 -Статистика

Статистика LiveInternet.ru: показано количество хитов и посетителей
Создан: 17.03.2011
Записей:
Комментариев:
Написано: 51


27000 ошибок в операционной системе Tizen

Среда, 12 Июля 2017 г. 11:13 + в цитатник
PVS-Studio and Tizen

Эта статья продемонстрирует, что при разработке крупных проектов статический анализ кода является не просто полезным, а совершенно необходимым элементом процесса разработки. Я начинаю цикл статей, посвященных возможности использования статического анализатора кода PVS-Studio для повышения качества и надежности операционной системы Tizen. Для начала я проверил небольшую часть операционной системы (3.3%) и выписал около 900 предупреждений, указывающих на настоящие ошибки. Если экстраполировать результаты, то получается, что наша команда способна выявить и устранить в Tizen около 27000 ошибок. По итогам проведённого исследования я подготовил презентацию, которая предназначалась для демонстрации представителям Samsung и была посвящена возможному сотрудничеству. Встреча перенесена на неопределённый срок, поэтому я решил не тратить время и трансформировать материал презентации в статью. Запасайтесь вкусняшками и напитками, нас ждёт длинный программистский триллер.

Начать, пожалуй, следует со ссылки на презентацию «Как команда PVS-Studio может улучшить код операционной системы Tizen», из которой и родилась эта статья: RU — pptx, slideshare; EN — pptx, slideshare. Впрочем, смотреть презентацию вовсе не обязательно, так как весь содержащийся в ней материал будет рассмотрен здесь, причём более подробно. Тема презентации пересекается с открытым письмом, в котором мы также рассказываем, что готовы поработать с проектом Tizen.

С разными воспоминаниями и ссылками я закончил и перехожу к сути дела. Первое, что стоит сделать, это напомнить читателю, что вообще из себя представляет операционная система Tizen.

Tizen


Tizen — открытая операционная система на базе ядра Linux, предназначенная для широкого круга устройств, включая смартфоны, интернет-планшеты, компьютеры, автомобильные информационно-развлекательные системы, «умные» телевизоры и цифровые камеры, разрабатываемая и управляемая такими корпорациями, как Intel и Samsung. Поддерживает аппаратные платформы на процессорах архитектур ARM и x86. Более подробная информация доступна на сайте Wikipedia.

Платформа Tizen показывает уверенный рост в течение нескольких последних лет, несмотря на насыщенность рынка операционных систем для мобильных и носимых устройств. Согласно отчёту Samsung, рост продаж мобильных телефонов с OC Tizen составил 100% в 2017 году.

Tizen


Для нашей команды операционная система Tizen привлекательна тем, что компания Samsung заинтересована в её надёжности и прикладывает усилия для улучшения качества кода. Например, с целью улучшения качества кода, Samsung инвестировал разработку в ИСП РАН специализированного анализатора кода Svace. Инструмент Svace используется как основное средство для обеспечения безопасности системного и прикладного ПО платформы Tizen. Вот некоторые цитаты, взятые из заметки «Samsung have Invested $10 Million in Svace, Security Solution to Analyze Tizen Apps»:

As part of its security measures, Samsung are using the SVACE technology (Security Vulnerabilities and Critical Errors Detector) to detect potential vulnerabilities and errors that might exist in source code of applications created for the Tizen Operating System (OS). This technology was developed by ISP RAS (Institute for System Programming of the Russian Academy of Sciences), who are based in Moscow, Russia.

The solution is applied as part of the Tizen Static Analyzer tool that is included in the Tizen SDK and Studio. Using this tool you can perform Static security analysis of the Tizen apps native C / C ++ source code and discover any issues that they might have. The tool helps discover a wide range of issues at compilation time, such as the dereference of Null Pointers, Memory Leaks, Division by Zero, and Double Free etc.

Также стоит отметить новость «В России впервые создали защищенную операционную систему», опубликованную на сайте rbc.ru. Цитата:

О создании российской версии операционной системы (ОС) Tizen было объявлено 2 июня 2016. Tizen является независимой ОС, то есть она не привязана к облаку Apple или Google, и поэтому ее можно адаптировать под локальные рынки, рассказал президент ассоциации «Тайзен.ру» Андрей Тихонов. Основная особенность представленной ОС — в ее безопасности, уточнил Тихонов. Это первая и единственная в России сертифицированная в Федеральной службе по техническому и экспортному контролю (ФСТЭК) мобильная операционная система, уточнил представитель Samsung.

Команда PVS-Studio просто не могла пройти мимо такого интересного открытого проекта, не попробовав его проверить.

Анализ Tizen


Целью презентации, о которой я упоминал ранее, было продемонстрировать, что анализатор PVS-Studio находит очень много ошибок различного типа. Это своего рода резюме анализатора и нашей команды, которое мы хотим показать компании Samsung.

Читатель может засомневаться, стоит ли ему читать такую «статью-резюме». Да, стоит, здесь будет много интересного и полезного. Во-первых, лучше учиться на чужих ошибках, а не на своих. Во-вторых, статья отлично показывает, что методологию статического анализа просто необходимо применять в проектах большого размера. Если кто-то из коллег, занятых в большом проекте, уверяет вас, что они пишут код высокого качества и почти без ошибок, то покажите ему эту статью. Я не думаю, что создатели Tizen хотели, чтобы в проекте было много дефектов, но вот они — тысячи багов.

Как всегда, хочу напомнить, что статический анализ кода должен применяться регулярно. Разовая проверка Tizen или любого другого проекта, конечно, будет полезна, но малоэффективна. В основном будут найдены малозначительные ошибки, которые слабо оказывают влияние на работоспособность проекта. Все явные ошибки были выявлены другими методами, например, благодаря жалобам пользователей. Означает ли это, что толку от статического анализа мало? Конечно нет, просто, как я уже сказал, разовые проверки — это неэффективный способ использования анализаторов кода. Анализаторы должны использоваться регулярно и тогда многие, в том числе и критические, ошибки будут обнаруживаться на самом раннем этапе. Чем раньше ошибка выявлена, тем дешевле её исправить.

Я считаю, что:
  • Сейчас анализатор PVS-Studio выявляет более 10% ошибок, которые присутствуют в коде проекта Tizen.
  • В случае регулярного использования PVS-Studio в новом коде можно будет предотвратить около 20% ошибок.
  • Я прогнозирую, что команда PVS-Studio может на данный момент выявить и исправить около 27000 ошибок в проекте Tizen.

Конечно, я могу ошибаться, но я не подгонял результаты исследования, чтобы показать возможности анализатора PVS-Studio в лучшем свете. Это просто не нужно. Анализатор PVS-Studio — мощный инструмент, который находит так много дефектов, что просто нет смысла фальсифицировать результаты. Далее я продемонстрирую, как получились все приведённые здесь числа.

Конечно, я не мог проверить весь проект Tizen. Вместе со сторонними библиотеками он насчитывает 72 500 000 строк C и C++ кода (без учёта комментариев). Поэтому я решил случайным образом проверить несколько десятков проектов, входящих в состав Tizen:Unified (https://build.tizen.org/project/show/Tizen:Unified).

Выбирая проекты, я делил их на две группы. Первая группа — это проекты, написанные непосредственно сотрудниками компании Samsung. Я определял такие проекты по наличию в начале файлов комментариев, выглядящих вот так:
/*
* Copyright (c) 2015 Samsung Electronics Co., Ltd All Rights Reserved
....
*/

Вторая группа — это сторонние проекты, используемые в проекте Tizen. Впрочем, многие проекты нельзя считать полностью сторонними, так как в них присутствуют различные патчи. Вот пример патча, сделанного в библиотеке efl-1.16.0:
//TIZEN_ONLY(20161121)
// Pre-rotation should be enabled only when direct
// rendering is set but client side rotation is not set
if ((sfc->direct_fb_opt) &&
    (!sfc->client_side_rotation) &&
    (evgl_engine->funcs->native_win_prerotation_set))
  {
    if (!evgl_engine->funcs->native_win_prerotation_set(eng_data))
      ERR("Prerotation does not work");
  }
//

Так что деление получилось несколько условным, однако, точность для общей оценки и не требуется.

Выбранные случайным образом проекты были проверены, и я приступил к изучению отчётов анализатора и выбору предупреждений, которые заслуживают внимания. Конечно, многие ошибки достаточно безобидны или могут проявлять себя крайне редко. Например, следующий код будет давать сбой очень-очень редко:
m_ptr = (int *)realloc(m_ptr, newSize);
if (!m_ptr) return ERROR;

Если не удастся выделить новый фрагмент памяти, то произойдёт утечка памяти. Подробнее этот тип ошибки будет рассмотрен далее. Да, вероятность утечки памяти крайне мала, но, на мой взгляд, это именно ошибка, которую следует исправить.

У меня ушло около недели на то, чтобы отобрать предупреждения, которые, по моему мнению, указывают на настоящие ошибки. Заодно я выписал большое количество примеров кода, которые я буду использовать для подготовки презентаций и статей.

Прошу внимания. Далее речь идёт именно о количестве ошибок, а не о количестве сообщений анализатора. Под ошибками я понимаю такие участки кода, которые, на мой взгляд, заслуживают того, чтобы их исправили.

Один из разработчиков, посмотрев презентацию и не разобравшись что к чему, написал комментарий вида «27000 предупреждений анализатора, тоже мне достижение, это даже мало». Поэтому ещё раз подчеркну, что речь идет именно об ошибках. В процессе исследования я считал и выписывал именно ошибки, а не просто все предупреждения анализатора без разбора.

Анализ проектов, разработанных специалистами компании Samsung


Для проверки я случайным образом выбрал следующие проекты: bluetooth-frwk-0.2.157, capi-appfw-application-0.5.5, capi-base-utils-3.0.0, capi-content-media-content-0.3.10, capi-maps-service-0.6.12, capi-media-audio-io-0.3.70, capi-media-codec-0.5.3, capi-media-image-util-0.1.15, capi-media-player-0.3.58, capi-media-screen-mirroring-0.1.78, capi-media-streamrecorder-0.0.10, capi-media-vision-0.3.24, capi-network-bluetooth-0.3.4, capi-network-http-0.0.23, cynara-0.14.10, e-mod-tizen-devicemgr-0.1.69, ise-engine-default-1.0.7, ise-engine-sunpinyin-1.0.10, ise-engine-tables-1.0.10, isf-3.0.186, org.tizen.app-selector-0.1.61, org.tizen.apps-0.3.1, org.tizen.bluetooth-0.1.2, org.tizen.browser-3.2.0, org.tizen.browser-profile_common-1.6.4, org.tizen.classic-watch-0.0.1, org.tizen.d2d-conv-setting-profile_mobile-1.0, org.tizen.d2d-conv-setting-profile_wearable-1.0, org.tizen.download-manager-0.3.21, org.tizen.download-manager-0.3.22, org.tizen.dpm-toolkit-0.1, org.tizen.elm-demo-tizen-common-0.1, org.tizen.indicator-0.2.53, org.tizen.inputdelegator-0.1.170518, org.tizen.menu-screen-1.2.5, org.tizen.myplace-1.0.1, org.tizen.privacy-setting-profile_mobile-1.0.0, org.tizen.privacy-setting-profile_wearable-1.0.0, org.tizen.quickpanel-0.8.0, org.tizen.screen-reader-0.0.8, org.tizen.service-plugin-sample-0.1.6, org.tizen.setting-1.0.1, org.tizen.settings-0.2, org.tizen.settings-adid-0.0.1, org.tizen.telephony-syspopup-0.1.6, org.tizen.voice-control-panel-0.1.1, org.tizen.voice-setting-0.0.1, org.tizen.volume-0.1.149, org.tizen.w-home-0.1.0, org.tizen.w-wifi-1.0.229, org.tizen.watch-setting-0.0.1, security-manager-1.2.17.

Проектов немало, но многие из них имеют совсем маленький размер. Давайте посмотрим, какие типы ошибок мне встретились.

Примечание. Помимо предупреждения PVS-Studio я буду стараться классифицировать найденные ошибки согласно CWE (Common Weakness Enumeration). Однако, я вовсе не делаю попытку найти какие-то уязвимости, а привожу CWE-ID исключительно для удобства тех читателей, которые привыкли именно к этой классификации дефектов. Моя основная цель — найти как можно больше ошибок, а определение того, насколько опасна ошибка с точки зрения безопасности, выходит за рамки моего исследования.

Это будет большой раздел, поэтому предлагаю заварить первую кружку чая или кофе. Вторая нам понадобится позже, так как мы только в самом начале статьи.

Чай, кофе


Опечатка в условии: слева и справа одно и то же (2 ошибки)


Классика. Даже так: классика самого высшего сорта!

Во-первых, эта ошибка выявляется с помощью диагностики V501. Диагностика выявляет опечатки и последствия неудачного Copy-Paste. Это невероятно популярный и распространенный тип ошибок. Посмотрите сами, какую замечательную коллекцию багов в открытых проектах мы собрали благодаря диагностике V501.

Во-вторых, эта ошибка находится в операторе меньше (< ). Неправильное сравнение двух объектов — это тоже классическая ошибка, возникающая из-за того, что никто не проверяет такие простые функции. Недавно я написал интересную статью на эту тему "Зло живёт в функциях сравнения". Почитайте, это своего рода «Хребты безумия» для программистов.

Код, о котором идёт речь:
bool operator <(const TSegment& other) const {
  if (m_start < other.m_start)
    return true;
  if (m_start == other.m_start)
    return m_len < m_len;                // <=
  return false;
}

Ошибка найдена благодаря предупреждению PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '<' operator: m_len < m_len segmentor.h 65

Software weaknesses type — CWE-570: Expression is Always False

Из-за этой ошибки объекты, которые отличаются только значением члена m_len, будут сравниваться некорректно. Правильный вариант сравнения:
return m_len < other.m_len;

Аналогичная ошибка: V501 There are identical sub-expressions '0 == safeStrCmp(btn_str, setting_gettext(«IDS_ST_BUTTON_OK»))' to the left and to the right of the '||' operator. setting-common-general-func.c 919

Опечатка в условии: бессмысленное сравнение (2 ошибки)


static void __page_focus_changed_cb(void *data)
{
  int i = 0;
  int *focus_unit = (int *)data;
  if (focus_unit == NULL || focus_unit < 0) {    // <=
    _E("focus page is wrong");
    return ;
  }
  ....
}

Ошибка найдена благодаря предупреждению PVS-Studio: V503 This is a nonsensical comparison: pointer < 0. apps_view_circle_indicator.c 193

Software weaknesses type — CWE-697: Insufficient Comparison

Сравнение вида «pointer < 0» не имеет смысла и свидетельствует об опечатке в коде. Как я понимаю, пропущен оператор звездочка (*), который должен был разыменовать указатель. Корректный код:
if (focus_unit == NULL || *focus_unit < 0) {

Этот код вместе с ошибкой скопировали, в результате чего точно такую же ошибку можно наблюдать в функции __page_count_changed_cb:
static void __page_count_changed_cb(void *data)
{
  int i = 0;
  int *page_cnt = (int *)data;
  if (page_cnt == NULL || page_cnt < 0) {
    _E("page count is wrong");
    return ;
  }
  ....
}

Ох уж эта Copy-Paste технология. Для этого кода анализатор выдал предупреждение: V503 This is a nonsensical comparison: pointer < 0. apps_view_circle_indicator.c 219

Опасный способ использования функции alloca (1 ошибка)


Рассмотрим код, который хотя и плох, но на практике не приведёт к проблемам. Я не рассматривал эту ситуацию в презентации, так как она требует специального пояснения. Теперь же, в статье, я могу остановиться на данном примере и поделиться своими мыслями.
int audio_io_loopback_in_test()
{
  ....
  while (1) {
    char *buffer = alloca(size);
    if ((ret = audio_in_read(input, (void *)buffer, size)) >
        AUDIO_IO_ERROR_NONE)
    {
      fwrite(buffer, size, sizeof(char), fp);
      printf("PASS, size=%d, ret=0x%x\n", size, ret);
    } else {
      printf("FAIL, size=%d, ret=0x%x\n", size, ret);
    }
  }
  ....
}

Предупреждение PVS-Studio: V505 The 'alloca' function is used inside the loop. This can quickly overflow stack. audio_io_test.c 247

Software weaknesses type — CWE-770: Allocation of Resources Without Limits or Throttling

В цикле, который выполняется, пока не закончится аудио-поток, происходит выделение стековой памяти с помощью функции alloca. Такой код плох тем, что очень быстро может исчерпаться стековая память.

Однако я не могу сказать, что нашел серьезную ошибку. Дело в том, что этот код относится к тестам. Уверен, что звуковой поток в тестах короткий и никаких проблем при его обработке в тестах не возникает.

Таким образом, не совсем честно говорить, что это именно ошибка, ведь тесты работают.

Но я и не согласен с тем, чтобы назвать это предупреждение ложным. Ведь код действительно плох. Через некоторое время кто-то может захотеть выполнять тесты на данных чуть большего размера, и произойдёт сбой. При этом поток данных должен стать не таким уж и большим. Достаточно, чтобы обрабатывались данные размером, равным размеру свободного стека, а это, как правило, совсем не много.

Более того, код можно легко исправить, а значит — это надо сделать. Достаточно вынести выделение памяти из цикла. Это можно сделать, так как размер выделяемого буфера не изменяется.

Хороший код:
char *buffer = alloca(size);
while (1) {
  if ((ret = audio_in_read(input, (void *)buffer, size)) >
      AUDIO_IO_ERROR_NONE)
  {
    fwrite(buffer, size, sizeof(char), fp);
    printf("PASS, size=%d, ret=0x%x\n", size, ret);
  } else {
    printf("FAIL, size=%d, ret=0x%x\n", size, ret);
  }
}

Используется уже несуществующий буфер (1 ошибка)


Следующий код также относится к тестам, но ошибка в нем гораздо серьезнее. Из-за ошибки возникает неопределённое поведение программы, поэтому доверять такому тесту совершенно нельзя. Другими словами — тест ничего не тестирует.
void extract_input_aacdec_m4a_test(
  App * app, unsigned char **data, int *size, bool * have_frame)
{
  ....
  unsigned char buffer[100000];
  ....
DONE:
  *data = buffer;
  *have_frame = TRUE;
  if (read_size >= offset)
    *size = offset;
  else
    *size = read_size;
}

Ошибка найдена благодаря предупреждению PVS-Studio: V507 Pointer to local array 'buffer' is stored outside the scope of this array. Such a pointer will become invalid. media_codec_test.c 793

Software weaknesses type — CWE-562: Return of Stack Variable Address

Функция возвращает адрес массива, созданного на стеке. После завершения функции этот массив будет уничтожен, и возвращенный из функции адрес никак нельзя использовать.

Обрабатывается меньше или больше элементов массива, чем требуется (7 ошибок)


Для начала рассмотрим случай, когда обрабатывается меньше элементов, чем требуется.
typedef int gint;
typedef gint gboolean;  

#define BT_REQUEST_ID_RANGE_MAX 245

static gboolean req_id_used[BT_REQUEST_ID_RANGE_MAX];

void _bt_init_request_id(void)
{
  assigned_id = 0;
  memset(req_id_used, 0x00, BT_REQUEST_ID_RANGE_MAX);
}

Предупреждение PVS-Studio: V512 A call of the 'memset' function will lead to underflow of the buffer 'req_id_used'. bt-service-util.c 38

Software weaknesses type — CWE-131: Incorrect Calculation of Buffer Size

Здесь программист забыл, что в качестве третьего аргумента функция memset принимает размер буфера в байтах, а не количество элементов массива. Не зря я в своё время назвал memset самой опасной функцией в мире программирования на C/C++. Эта функция продолжает сеять хаос в различных проектах.

Тип gboolean занимает не 1 байт, а 4. В результате будет обнулена только 1/4 часть массива, а остальные элементы останутся неинициализированными.

Правильный вариант кода:
memset(req_id_used, 0x00, BT_REQUEST_ID_RANGE_MAX * sizeof(gboolean));

Или можно написать так:
memset(req_id_used, 0x00, sizeof(req_id_used));

Теперь рассмотрим случай, когда может произойти выход за границу массива.
static void _on_atspi_event_cb(const AtspiEvent * event)
{
  ....
  char buf[256] = "\0";
  ....
  snprintf(buf, sizeof(buf), "%s, %s, ",
           name, _("IDS_BR_BODY_IMAGE_T_TTS"));
  ....
  snprintf(buf + strlen(buf), sizeof(buf),
           "%s, ", _("IDS_ACCS_BODY_SELECTED_TTS"));
  ....
}

Предупреждение PVS-Studio: V512 A call of the 'snprintf' function will lead to overflow of the buffer 'buf + strlen(buf)'. app_tracker.c 450

Software weaknesses type — CWE-131: Incorrect Calculation of Buffer Size

Надежная операционная система… Эх…

Обратите внимание, что второй вызов snprintf должен добавить что-то к уже имеющейся строке. Поэтому адресом буфера является выражение buf + strlen(buf). При этом функция имеет право печатать символов меньше, чем размер буфера. Из размера буфера надо вычесть strlen(buf). Но про это забыли и может возникнуть ситуация, когда функция snprintf запишет данные за границу массива.

Корректный код:
snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
         "%s, ", _("IDS_ACCS_BODY_SELECTED_TTS"));

Третий фрагмент кода демонстрирует ситуацию, когда выход за границу происходит всегда. Для начала рассмотрим некоторые структуры.
#define BT_ADDRESS_STRING_SIZE 18

typedef struct {
 unsigned char addr[6];
} bluetooth_device_address_t;

typedef struct {
 int count;
 bluetooth_device_address_t addresses[20];
} bt_dpm_device_list_t;

Здесь нам важно, что массив addr состоит из 6 элементов. Запомните этот размер, а также, что макрос BT_ADDRESS_STRING_SIZE раскрывается в константу 18.

Теперь собственно некорректный код:
dpm_result_t _bt_dpm_get_bluetooth_devices_from_whitelist(
  GArray **out_param1)
{
  dpm_result_t ret = DPM_RESULT_FAIL;
  bt_dpm_device_list_t device_list;
  ....
  for (; list; list = list->next, i++) {
    memset(device_list.addresses[i].addr, 0, BT_ADDRESS_STRING_SIZE);
    _bt_convert_addr_string_to_type(device_list.addresses[i].addr,
                                    list->data);
  }
  ....
}

Предупреждение PVS-Studio: V512 A call of the 'memset' function will lead to overflow of the buffer 'device_list.addresses[i].addr'. bt-service-dpm.c 226

Software weaknesses type — CWE-805: Buffer Access with Incorrect Length Value

Выделю самое главное: 
memset(device_list.addresses[i].addr, 0, BT_ADDRESS_STRING_SIZE);

Итак, как мы видели раньше, размер addr всего 6 байт. При этом функция memset обнуляет 18 байт и, как следствие, происходит выход за границу массива.

Ещё 4 найденные ошибки:
  • V512 A call of the 'memset' function will lead to overflow of the buffer 'device_list.addresses[i].addr'. bt-service-dpm.c 176
  • V512 A call of the 'memset' function will lead to underflow of the buffer 'formatted_number'. i18ninfo.c 544
  • V512 A call of the 'snprintf' function will lead to overflow of the buffer 'ret + strlen(ret)'. navigator.c 677
  • V512 A call of the 'snprintf' function will lead to overflow of the buffer 'trait + strlen(trait)'. navigator.c 514

Логическая ошибка в последовательностях if… else… if (4 ошибки)


char *voice_setting_language_conv_lang_to_id(const char* lang)
{
  ....
  } else if (!strcmp(LANG_PT_PT, lang)) {
    return "pt_PT";
  } else if (!strcmp(LANG_ES_MX, lang)) {     // <=
    return "es_MX";
  } else if (!strcmp(LANG_ES_US, lang)) {     // <=
    return "es_US";
  } else if (!strcmp(LANG_EL_GR, lang)) {
    return "el_GR";
  ....
}

Предупреждение PVS-Studio: V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 144, 146. voice_setting_language.c 144

Software weaknesses type — CWE-570 Expression is Always False

Рассматривая этот код, нельзя понять, в чём же заключается ошибка. Всё дело в том, что LANG_ES_MX и LANG_ES_US — это идентичные строки. Вот они:
#define LANG_ES_MX "\x45\x73\x70\x61\xC3\xB1\x6f\x6c\x20\x28\" \
 "x45\x73\x74\x61\x64\x6f\x73\x20\x55\x6e\x69\x64\x6f\x73\x29"

#define LANG_ES_US "\x45\x73\x70\x61\xC3\xB1\x6f\x6c\x20\x28\" \
 "x45\x73\x74\x61\x64\x6f\x73\x20\x55\x6e\x69\x64\x6f\x73\x29"

Как я понимаю, эти строки на самом деле должны отличаться. Но поскольку строки одинаковые, то второе условие всегда ложно и функция никогда не вернёт значение «es_US».

Примечание. ES_MX — это Spanish (Mexico), ES_US — это Spanish (United States).

Эту ошибку я нашел в проекте org.tizen.voice-setting-0.0.1. Что интересно, вновь подводит технология Copy-Paste, и точно такую же ошибку я встретил в проекте org.tizen.voice-control-panel-0.1.1.

Другие ошибки:
  • V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 144, 146. voice_setting_language.c 144
  • V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 792, 800. setting-common-general-func.c 792
  • V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 801, 805. setting-common-general-func.c 801

Повторное присваивание (11 ошибок)


Рассмотрим ошибку в логике работы программы. Разработчик хотел обменять значения двух переменных, но запутался и у него получился следующий код:
void
isf_wsc_context_del (WSCContextISF *wsc_ctx)
{
  ....
  WSCContextISF* old_focused = _focused_ic;
  _focused_ic = context_scim;
  _focused_ic = old_focused;
  ....
}

Предупреждение PVS-Studio: V519 The '_focused_ic' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1260, 1261. wayland_panel_agent_module.cpp 1261

Software weaknesses type — CWE-563 Assignment to Variable without Use ('Unused Variable')

Переменной _focused_ic два раза подряд присваиваются разные значения. Правильный код должен быть таким:
WSCContextISF* old_focused = _focused_ic;
_focused_ic = context_scim;
context_scim = old_focused;

Впрочем, в таких случаях лучше использовать функцию std::swap. Так намного меньше шансов ошибиться:
std::swap(_focused_ic, context_scim);

Рассмотрим другой вариант ошибки, которая возникла при написании однотипного кода. Возможно, в её появлении виноват метод Copy-Paste. 
void create_privacy_package_list_view(app_data_s* ad)
{
  ....
  Elm_Genlist_Item_Class *ttc = elm_genlist_item_class_new();
  Elm_Genlist_Item_Class *ptc = elm_genlist_item_class_new();
  Elm_Genlist_Item_Class *mtc = elm_genlist_item_class_new();
  ....
  ttc->item_style = "title";
  ttc->func.text_get = gl_title_text_get_cb;
  ttc->func.del = gl_del_cb;                   // <=

  ptc->item_style = "padding";
  ptc->func.del = gl_del_cb;

  mtc->item_style = "multiline";
  mtc->func.text_get = gl_multi_text_get_cb;
  ttc->func.del = gl_del_cb;                   // <=
  ....
}

Предупреждение PVS-Studio: V519 The 'ttc->func.del' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 409, 416. privacy_package_list_view.c 416

Software weaknesses type — CWE-563 Assignment to Variable without Use ('Unused Variable')

Во втором случае значение должно было присваиваться переменной mtc->func.del.

Другие ошибки:
  • V519 The 'item' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1176, 1189. w-input-stt-voice.cpp 1189
  • V519 The 'ad->paired_item' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1614, 1617. bt-main-view.c 1617
  • V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 191, 194. main.c 194
  • V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 372, 375. setting-about-status.c 375
  • V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 91, 100. setting-applications-defaultapp.c 100
  • V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 52, 58. smartmanager-battery.c 58
  • V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 853, 872. setting-time-main.c 872
  • V519 The 'ret_str' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2299, 2300. setting-password-sim.c 2300
  • V519 The 'display_status' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 110, 117. view_clock.c 117

При просмотре отчета анализатора я выделил только 11 мест, которые, на мой взгляд, следует исправить. Но на самом деле сообщений V519 было намного больше. Очень часто они относились к коду, когда в переменную много раз подряд сохранялся результат после вызова функций. Речь идёт о коде вида:
status = Foo(0); 
status = Foo(1);
status = Foo(2);

Такой код возникает, как правило, в двух случаях:
  1. Результат вызова функций не важен, но какой-то компилятор или анализатор выдал предупреждение, от которого избавились, записав результат в переменную. По-моему мнению, тогда было бы лучше написать (void)Foo(x);.
  2. Результат вызова функций не важен, но иногда может пригодиться для отладки. Намного проще отлаживать код, когда результат записывается в какую-то переменную.

Про этот момент я пишу, так как, возможно, не везде такой код безопасен, как кажется на первый взгляд. Вероятно, в некоторых местах результат, который вернули функции, зря обделён вниманием и не проверен. Я смотрел код быстро и не разбирался, как и что устроено. Думаю, если подойти к этим предупреждениям внимательнее, то можно будет обнаружить гораздо больше дефектов.

Разыменование (потенциально) нулевого указателя (всего ошибок 88)


Использование нулевых указателей выявляется с помощью диагностик V522 и V575. Предупреждение V522 выдаётся, кода происходит разыменование указателя, который может быть нулевым (*MyNullPtr = 2;). V575 — когда потенциально нулевой указатель передаётся в функцию, внутри которой он может быть разыменован (s = strlen(MyNullPtr);). На самом деле V575 выдаётся и для некоторых других случаев, когда используются некорректные аргументы, но на данный момент это нам не интересно. Сейчас, с точки зрения статьи, разницы между предупреждениями V522 и V575 нет, поэтому они будут рассмотрены в этой главе совместно.

Отдельно стоит поговорить о таких функциях, как malloc, realloc, strdup. Следует проверять, не вернули ли они NULL, если невозможно выделить блок памяти запрошенного размера. Впрочем, некоторые программисты придерживаются плохой практики и никогда сознательно не делают проверок. Их логика такова, раз нет памяти, то и не стоит дальше мучиться, пусть программа упадёт. Я считаю такой подход плохим, но он есть, и я не раз слышал доводы в его защиту.

К счастью, разработчики Tizen к упомянутой категории не относятся и обычно проверяют, удалось ли выделить память или нет. Иногда они это делают даже там, где не надо:
static FilterModule *__filter_modules = 0;
static void
__initialize_modules (const ConfigPointer &config)
{
  ....
  __filter_modules = new FilterModule [__number_of_modules];
  if (!__filter_modules) return;
  ....
}

В такой проверке нет смысла, так как в случае неудачи выделения памяти оператор new сгенерирует исключение типа std::bad_alloc. Впрочем, это совсем другая история. Я привел этот код исключительно для того, чтобы показать, что в проекте Tizen принято проверять, удалось ли выделить память.

Так вот, анализатор PVS-Studio обнаруживает, что во многих местах проверок не хватает. Здесь я рассмотрю только один случай, так как в общем-то они все однотипны.
void QuickAccess::setButtonColor(Evas_Object* button,
                                 int r, int g, int b, int a)
{
  Edje_Message_Int_Set* msg =
  (Edje_Message_Int_Set *)malloc(sizeof(*msg) + 3 * sizeof(int));
  msg->count = 4;
  msg->val[0] = r;
  msg->val[1] = g;
  msg->val[2] = b;
  msg->val[3] = a;
  edje_object_message_send(elm_layout_edje_get(button),
                           EDJE_MESSAGE_INT_SET, 0, msg);
  free(msg);
}

Предупреждение PVS-Studio: V522 There might be dereferencing of a potential null pointer 'msg'. QuickAccess.cpp 743

Software weaknesses type — CWE-690: Unchecked Return Value to NULL Pointer Dereference

Нет гарантии, что функция malloc выделит память. Да, вероятность такого события крайне мала, но если проверки на равенство указателей NULL есть в других местах, то проверка должна быть и здесь. Поэтому я считаю, что этот код содержит настоящую ошибку, которую необходимо исправить.

Однако нулевые указатели могут возвращать не только функции, выделяющие память. Есть и другие ситуации, когда следует проверять указатель перед использованием. Рассмотрим пару таких примеров. Первый из них связан с небезопасным использованием оператора dynamic_cast.
int cpp_audio_in_peek(audio_in_h input, const void **buffer,
                      unsigned int *length) {
  ....
  CAudioInput* inputHandle = 
    dynamic_cast(handle->audioIoHandle);
  assert(inputHandle);
  inputHandle->peek(buffer, &_length);
  ....
}

Предупреждение PVS-Studio: V522 There might be dereferencing of a potential null pointer 'inputHandle'. cpp_audio_io.cpp 928

Software weaknesses type — CWE-690: Unchecked Return Value to NULL Pointer Dereference

Странный код. Если есть уверенность, что в handle->audioIoHandle хранится указатель на объект типа CAudioInput, то надо использовать static_cast. Если уверенности нет, то нужна проверка, так как макрос assert не поможет в release-версии.

Думаю, правильнее будет добавить вот такую проверку:
CAudioInput* inputHandle = 
  dynamic_cast(handle->audioIoHandle);
if (inputHandle == nullptr) {
  assert(false);
  THROW_ERROR_MSG_FORMAT(
     CAudioError::EError::ERROR_INVALID_HANDLE, "Handle is NULL");
}

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

Следующий код, возможно, и не приводит к настоящей ошибке. Допустим, сейчас всегда обрабатываются такие строки, в которых обязательно присутствуют символы '-' и '.'. Однако, я надеюсь вы согласитесь, что код опасен и лучше подстраховаться. Выбрал я его, чтобы продемонстрировать разнообразие ситуаций, когда анализатор выдаёт предупреждения.
int main(int argc, char *argv[])
{
  ....
  char *temp1 = strstr(dp->d_name, "-");
  char *temp2 = strstr(dp->d_name, ".");

  strncpy(temp_filename, dp->d_name,
          strlen(dp->d_name) - strlen(temp1));
  strncpy(file_format, temp2, strlen(temp2));
  ....
}

Предупреждения PVS-Studio:
  • V575 The potential null pointer is passed into 'strlen' function. Inspect the first argument. image_util_decode_encode_testsuite.c 207
  • V575 The potential null pointer is passed into 'strlen' function. Inspect the first argument. image_util_decode_encode_testsuite.c 208

Software weaknesses type — CWE-690: Unchecked Return Value to NULL Pointer Dereference

Указатели temp1 и temp2 могут оказаться нулевыми, если в строке не обнаружатся символы '-' и '.'. В этом случае чуть позже произойдёт разыменование нулевого указателя.

Есть ещё 84 участка кода, где разыменовываются указатели, которые могут оказаться равны NULL. Рассматривать их в статье нет никакого смысла. Нет смысла даже приводить их просто списком, так как это все равно займёт много места. Поэтому я выписал эти предупреждения в отдельный файл: Tizen_V522_V575.txt.

Одинаковые действия (6 ошибок)


static void _content_resize(...., const char *signal)
{
  ....
  if (strcmp(signal, "portrait") == 0) {
    evas_object_size_hint_min_set(s_info.layout,
      ELM_SCALE_SIZE(width), ELM_SCALE_SIZE(height));
  } else {
    evas_object_size_hint_min_set(s_info.layout,
      ELM_SCALE_SIZE(width), ELM_SCALE_SIZE(height));
  }
  ....
}

Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. page_setting_all.c 125

Software weaknesses type — не знаю как классифицировать, буду благодарен за подсказку.

Вне зависимости от условия, выполняются одни и те же действия. Как я понимаю, в одном из двух вызовов функции evas_object_size_hint_min_set следует поменять местами width и height.

Рассмотрим ещё одну ошибку этого типа:
static Eina_Bool _move_cb(void *data, int type, void *event)
{
  Ecore_Event_Mouse_Move *move = event;

  mouse_info.move_x = move->root.x;
  mouse_info.move_y = move->root.y;

  if (mouse_info.pressed == false) {
    return ECORE_CALLBACK_RENEW;                 // <=
  }

  return ECORE_CALLBACK_RENEW;                   // <=
}

Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the subsequent code fragment. mouse.c 143

Software weaknesses type — CWE-393 Return of Wrong Status Code

Очень странно, что функция выполняет какую-то проверку, но всё равно всегда возвращает значение ECORE_CALLBACK_RENEW. Думаю, возвращаемые значения должны различаться.

Другие ошибки этого типа:
  • V523 The 'then' statement is equivalent to the 'else' statement. bt-httpproxy.c 383
  • V523 The 'then' statement is equivalent to the 'else' statement. streamrecorder_test.c 342
  • V523 The 'then' statement is equivalent to the 'else' statement. ise-stt-option.cpp 433
  • V523 The 'then' statement is equivalent to the subsequent code fragment. dpm-toolkit-cli.c 87

Забыли разыменовать указатель (1 ошибка)


Очень красивая ошибка: пишем данные неизвестно куда.
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

Software weaknesses type — CWE-787: Out-of-bounds Write

Функция принимает указатель на указатель. Это позволяет при необходимости перевыделить память и вернуть адрес новой строки.

Ошибка кроется в строке: 
body[new_len] = '\0';

Получается, что указатель на указатель интерпретируется как массив указателей. Никакого массива, конечно, нет. Поэтому NULL ('\0' в данном случае воспринимается как нулевой указатель) будет записан совершенно непонятно куда. Происходит порча какого-то неизвестного блока памяти.

Ох..


Помимо этого, возникает ещё одна ошибка. Строка не будет завершена терминальным нулём. В общем, всё плохо.

Правильный код:
(*body)[new_len] = '\0';

Условие всегда true/false (9 ошибок)


Есть много причин, приводящих к ошибке, из-за которой условие всегда является истинным или ложным, но в статье для краткости я рассмотрю только 3 варианта возникновения ошибки.

Первый вариант.
unsigned m_candiPageFirst;

bool
CIMIClassicView::onKeyEvent(const CKeyEvent& key)
{
  ....
  if (m_candiPageFirst > 0) {
    m_candiPageFirst -= m_candiWindowSize;
    if (m_candiPageFirst < 0) m_candiPageFirst = 0;
    changeMasks |= CANDIDATE_MASK;
  }
  ....
}

Предупреждение PVS-Studio: V547 Expression 'm_candiPageFirst < 0' is always false. Unsigned type value is never < 0. imi_view_classic.cpp 201

Software weaknesses type — CWE-570: Expression is Always False

Переменная m_candiPageFirst имеет тип unsigned. Следовательно, значение этой переменной не может быть меньше нуля. Чтобы защититься от переполнения, следует переписать код так:
if (m_candiPageFirst > 0) {
  if (m_candiPageFirst > m_candiWindowSize)
    m_candiPageFirst -= m_candiWindowSize;
  else 
    m_candiPageFirst = 0;
  changeMasks |= CANDIDATE_MASK;
}

Второй вариант.
void
QuickAccess::_grid_mostVisited_del(void *data, Evas_Object *)
{
  BROWSER_LOGD("[%s:%d]", __PRETTY_FUNCTION__, __LINE__);
  if (data) {
    auto itemData = static_cast(data);
    if (itemData)
      delete itemData;
  }
}

Предупреждение PVS-Studio: V547 Expression 'itemData' is always true. QuickAccess.cpp 571

Software weaknesses type — CWE-571: Expression is Always True

Это очень подозрительный код. Если указатель data != nullptr, то и указатель itemData != nullptr. Следовательно, вторая проверка не имеет смысла. Здесь мы столкнулись с одной из двух ситуаций:
  1. Это ошибка. На самом деле вместо оператора static_cast следовало использовать оператор dynamic_cast, который может вернуть nullptr.
  2. Настоящей ошибки нет, это неаккуратный код. Вторая проверка просто лишняя и её следует удалить, чтобы она не сбивала с толку анализаторы кода и других программистов.

Сейчас мне сложно сказать, следует выбрать пункт 1 или 2, но в любом случае, этот код заслуживает внимания и его следует исправить.

Третий вариант.
typedef enum {
  BT_HID_MOUSE_BUTTON_NONE = 0x00,
  BT_HID_MOUSE_BUTTON_LEFT = 0x01,
  BT_HID_MOUSE_BUTTON_RIGHT = 0x02,
  BT_HID_MOUSE_BUTTON_MIDDLE = 0x04
} bt_hid_mouse_button_e;

int bt_hid_device_send_mouse_event(const char *remote_address,
    const bt_hid_mouse_data_s *mouse_data)
{
  ....
  if (mouse_data->buttons != BT_HID_MOUSE_BUTTON_LEFT ||
      mouse_data->buttons != BT_HID_MOUSE_BUTTON_RIGHT ||
      mouse_data->buttons != BT_HID_MOUSE_BUTTON_MIDDLE) {
    return BT_ERROR_INVALID_PARAMETER;
  }
  ....
}

Предупреждение PVS-Studio: V547 Expression is always true. Probably the '&&' operator should be used here. bluetooth-hid.c 229

Software weaknesses type — CWE-571: Expression is Always True

Чтобы было проще понять в чем ошибка, я подставлю значения констант и сокращу код:
if (buttons != 1 ||
    buttons != 2 ||
    buttons != 4) {

Какое бы значение не хранилось в переменной, оно всегда будет не равно 1 или 2 или 4.

Другие ошибки:
  • V547 Expression 'ad->transfer_info' is always true. bt-share-ui-popup.c 56
  • V547 Expression 'item_name' is always true. SettingsUI.cpp 222
  • V547 Expression 'item_name' is always true. SettingsUI.cpp 226
  • V547 Expression '!urlPair' is always false. GenlistManager.cpp 143
  • V547 Expression 'strlen(s_formatted) < 128' is always true. clock.c 503
  • V547 Expression 'doc != ((void *) 0)' is always true. setting-common-data-slp-setting.c 1450

Путаница с enum (18 ошибок)


В презентации этот вид ошибки я пропустил, так как примеры слишком длинные. Но в статье, я думаю, есть смысл вспомнить про них.

Есть два enum, в которых объявлены константы с похожими именами:
typedef enum {
 WIFI_MANAGER_RSSI_LEVEL_0 = 0,
 WIFI_MANAGER_RSSI_LEVEL_1 = 1,
 WIFI_MANAGER_RSSI_LEVEL_2 = 2,
 WIFI_MANAGER_RSSI_LEVEL_3 = 3,
 WIFI_MANAGER_RSSI_LEVEL_4 = 4,
} wifi_manager_rssi_level_e;

typedef enum {
 WIFI_RSSI_LEVEL_0 = 0,
 WIFI_RSSI_LEVEL_1 = 1,
 WIFI_RSSI_LEVEL_2 = 2,
 WIFI_RSSI_LEVEL_3 = 3,
 WIFI_RSSI_LEVEL_4 = 4,
} wifi_rssi_level_e;

Неудивительно, что в именах можно запутаться и написать вот такой код:
static int
_rssi_level_to_strength(wifi_manager_rssi_level_e level)
{
  switch (level) {
    case WIFI_RSSI_LEVEL_0:
    case WIFI_RSSI_LEVEL_1:
      return LEVEL_WIFI_01;
    case WIFI_RSSI_LEVEL_2:
      return LEVEL_WIFI_02;
    case WIFI_RSSI_LEVEL_3:
      return LEVEL_WIFI_03;
    case WIFI_RSSI_LEVEL_4:
      return LEVEL_WIFI_04;
    default:
      return WIFI_RSSI_LEVEL_0;
  }
}

Переменная level имеет тип wifi_manager_rssi_level_e. Константы же имеют тип wifi_rssi_level_e. Получается, что есть сразу 5 неправильных сравнений, и поэтому анализатор выдаёт 5 предупреждений:
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. wifi.c 163
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. wifi.c 164
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. wifi.c 166
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. wifi.c 168
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. wifi.c 170

Software weaknesses type — CWE-697: Insufficient Comparison

Что забавно — этот код работает именно так, как задумывал программист. Благодаря везению, константа WIFI_MANAGER_RSSI_LEVEL_0 равна WIFI_RSSI_LEVEL_0 и так далее.

Несмотря на то, что сейчас код работает, это ошибка и её следует исправить. На это две причины:
  1. Этот код удивляет анализатор, а значит он будет удивлять и программистов, которые будут его сопровождать.
  2. Если хотя бы один из enum со временем изменится и значения констант перестанут совпадать, то программа начнет вести себя некорректно.

Другие неправильные сравнения:
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 885
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 889
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 892
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 895
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 898
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 901
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 904
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. e_devicemgr_video.c 907
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. myplace-placelist.c 239
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. myplace-placelist.c 253
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. myplace-placelist.c 264
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. telephony_syspopup_noti.c 82
  • V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. telephony_syspopup_noti.c 87

Часть условия всегда true/false (2 ошибки)


Я заметил всего 2 таких ошибки, но они обе интересные, поэтому давайте рассмотрим их.
int bytestream2nalunit(FILE * fd, unsigned char *nal)
{
  unsigned char val, zero_count, i;
  ....
  val = buffer[0];
  while (!val) {                                            // <=
    if ((zero_count == 2 || zero_count == 3) && val == 1)   // <=
      break;
    zero_count++;
    result = fread(buffer, 1, read_size, fd);

    if (result != read_size)
      break;
    val = buffer[0];
  }
  ....
}

Предупреждение PVS-Studio: V560 A part of conditional expression is always false: val == 1. player_es_push_test.c 284

Software weaknesses type — CWE-570: Expression is Always False

Цикл выполняется до тех пор, пока переменная val равна нулю. В начале тела цикла переменная val сравнивается со значением 1. Естественно, переменная val не может быть равна 1, иначе бы цикл уже остановился. Здесь явно какая-то логическая ошибка.

Теперь рассмотрим другую ошибку.
const int GT_SEARCH_NO_LONGER = 0,
          GT_SEARCH_INCLUDE_LONGER = 1,
          GT_SEARCH_ONLY_LONGER = 2;

bool GenericTableContent::search (const String &key,
                                  int search_type) const
{
  ....
  else if (nkeys.size () > 1 && GT_SEARCH_ONLY_LONGER) {
  ....
}

Предупреждение PVS-Studio: V560 A part of conditional expression is always true: GT_SEARCH_ONLY_LONGER. scim_generic_table.cpp 1884

Software weaknesses type — CWE-571: Expression is Always True

Константа GT_SEARCH_ONLY_LONGER является частью условия. Это очень странно и у меня есть подозрение, что на самом деле условие должно выглядеть так:
if (nkeys.size () > 1 && search_type == GT_SEARCH_ONLY_LONGER)

Путаница с типами создаваемых и уничтожаемых объектов (4 ошибки)


Объявлены три структуры, никак не связанные между собой:
struct sockaddr_un
{
  sa_family_t sun_family;
  char sun_path[108];
};

struct sockaddr_in
{
  sa_family_t sin_family;
  in_port_t sin_port;
  struct in_addr sin_addr;
  unsigned char sin_zero[sizeof (struct sockaddr) -
    (sizeof (unsigned short int)) -
    sizeof (in_port_t) -
    sizeof (struct in_addr)];
};

struct sockaddr
{
  sa_family_t sa_family;
  char sa_data[14];
};

Ошибка заключается в том, что создаются объекты одного типа, а уничтожаются они как объекты другого типа:
class SocketAddress::SocketAddressImpl
{
  struct sockaddr *m_data;
  ....
  SocketAddressImpl (const SocketAddressImpl &other)
  {
    ....
    case SCIM_SOCKET_LOCAL:
        m_data = (struct sockaddr*) new struct sockaddr_un; // <=
        len = sizeof (sockaddr_un);
        break;
    case SCIM_SOCKET_INET:
        m_data = (struct sockaddr*) new struct sockaddr_in; // <=
        len = sizeof (sockaddr_in);
        break;
    ....
  }

  ~SocketAddressImpl () {
    if (m_data) delete m_data;                              // <=
  }
};

Предупреждения анализатора:
  • V572 It is odd that the object which was created using 'new' operator is immediately cast to another type. scim_socket.cpp 136
  • V572 It is odd that the object which was created using 'new' operator is immediately cast to another type. scim_socket.cpp 140

Software weaknesses type — CWE-762: Mismatched Memory Management Routines.

Создаются структуры типа sockaddr_un и sockaddr_in. При этом хранятся и уничтожаются они как структуры типа sockaddr. Типы всех трёх названных структур никак не связаны между собой. Это три разные структуры, имеющие разный размер. Сейчас код вполне может работать, так как структуры являются POD типами (не содержат деструкторы и т.д.) и вызов оператора delete превращается в простой вызов функции free. Однако формально код неверен. Нужно уничтожать объект точно такого же типа, который использовался при создании объекта.

Как я уже сказал, сейчас программа может работать, хотя формально и является неправильной. Надо понимать, что рассмотренный код очень опасен и достаточно, чтобы в одном из классов появился конструктор/деструктор или был добавлен член сложного типа (например std::string), как всё сразу сломается окончательно.

Другие ошибки:
  • V572 It is odd that the object which was created using 'new' operator is immediately cast to another type. scim_socket.cpp 167
  • V572 It is odd that the object which was created using 'new' operator is immediately cast to another type. scim_socket.cpp 171

Неправильная работа с printf-подобными функциями (4 ошибки)


static int _write_file(const char *file_name, void *data,
                       unsigned long long data_size)
{
  FILE *fp = NULL;

  if (!file_name || !data || data_size <= 0) {
    fprintf(stderr,
            "\tinvalid data %s %p size:%lld\n",
            file_name, data, data_size);
    return FALSE;
  }
  ....
}

Предупреждение PVS-Studio: V576 Incorrect format. Consider checking the third actual argument of the 'fprintf' function. Under certain conditions the pointer can be null. image_util_decode_encode_testsuite.c 124

Software weaknesses type — CWE-476: NULL Pointer Dereference

Возможна ситуация, когда указатель file_name будет содержать NULL. Как в таком случае поведёт себя функция printf, предсказать невозможно. На практике поведение будет зависеть от используемой реализации функции printf. См. дискуссию "What is the behavior of printing NULL with printf's %s specifier?".

Рассмотрим ещё одну ошибку.
void subscribe_to_event()
{
  ....
  int error = ....;
  ....
  PRINT_E("Failed to destroy engine configuration for event trigger.",
          error);
  ....
}

Предупреждение PVS-Studio: V576 Incorrect format. A different number of actual arguments is expected while calling 'printf' function. Expected: 1. Present: 2. surveillance_test_suite.c 393

Software weaknesses type — не знаю как классифицировать, буду благодарен за подсказку.

Макрос PRINT_E раскрывается в printf. Как видите, переменная error никак не используется. Видимо номер ошибки забыли распечатать.

Другие ошибки:
  • V576 Incorrect format. A different number of actual arguments is expected while calling 'printf' function. Expected: 1. Present: 2. surveillance_test_suite.c 410
  • V576 Incorrect format. A different number of actual arguments is expected while calling 'printf' function. Expected: 1. Present: 2. surveillance_test_suite.c 417

Проверка указателя выполняется уже после его разыменования (5 ошибок)


static void _show(void *data)
{
  SETTING_TRACE_BEGIN;
  struct _priv *priv = (struct _priv *)data;
  Eina_List *children = elm_box_children_get(priv->box);    // <=
  Evas_Object *first = eina_list_data_get(children);
  Evas_Object *selected =
    eina_list_nth(children, priv->item_selected_on_show);   // <=

  if (!priv) {                                              // <=
    _ERR("Invalid parameter.");
    return;
  }
  ....
}

Предупреждение PVS-Studio: V595 The 'priv' pointer was utilized before it was verified against nullptr. Check lines: 110, 114. view_generic_popup.c 110

Software weaknesses type — CWE-476: NULL Pointer Dereference

Указатель priv дважды разыменовывается в выражениях:
  • priv->box
  • priv->item_selected_on_show

И только затем указатель проверяется на равенство нулю. Чтобы исправить код, проверку следует передвинуть выше:
static void _show(void *data)
{
  SETTING_TRACE_BEGIN;
  struct _priv *priv = (struct _priv *)data;
  if (!priv) {
    _ERR("Invalid parameter.");
    return;
  }
  Eina_List *children = elm_box_children_get(priv->box);
  Evas_Object *first = eina_list_data_get(children);
  Evas_Object *selected =
    eina_list_nth(children, priv->item_selected_on_show);
  ....
}

Теперь рассмотрим более сложный случай.

Есть функция _ticker_window_create, в которой разыменовывается указатель, переданный функции в качестве аргумента.
static Evas_Object *_ticker_window_create(struct appdata *ad)
{
  ....
  // нет проверки указателя 'ad'
  ....
  evas_object_resize(win, ad->win.w, indicator_height_get());
  ....
}

Важно отметить, что указатель разыменовывается без предварительной проверки на равенство NULL. Другими словами, в функцию _ticker_window_create можно передавать только ненулевые указатели. Теперь посмотрим, как эта функция используется на самом деле.
static int _ticker_view_create(void)
{
  if (!ticker.win)
    ticker.win = _ticker_window_create(ticker.ad);       // <=
  if (!ticker.layout)
    ticker.layout = _ticker_layout_create(ticker.win);
  if (!ticker.scroller)
    ticker.scroller = _ticker_scroller_create(ticker.layout);

  evas_object_show(ticker.layout);
  evas_object_show(ticker.scroller);
  evas_object_show(ticker.win);

  if (ticker.ad)                                         // <=
    util_signal_emit_by_win(&ticker.ad->win,
      "message.show.noeffect", "indicator.prog");
  ....
}

Предупреждение PVS-Studio: V595 The 'ticker.ad' pointer was utilized before it was verified against nullptr. Check lines: 590, 600. ticker.c 590

Software weaknesses type — CWE-476: NULL Pointer Dereference

Указатель ticker.ad передаётся в функцию _ticker_window_create. Ниже есть проверка "if (ticket.ad)", которая свидетельствует о том, что этот указатель может быть нулевым.

Другие ошибки:
  • V595 The 'core' pointer was utilized before it was verified against nullptr. Check lines: 2252, 2254. media_codec_port_gst.c 2252
  • V595 The 'eyeCondition' pointer was utilized before it was verified against nullptr. Check lines: 162, 168. FaceEyeCondition.cpp 162
  • V595 The 'dev->name' pointer was utilized before it was verified against nullptr. Check lines: 122, 127. e_devicemgr_device.c 122

Не затираются приватные данные (1 ошибка)


static void SHA1Final(unsigned char digest[20],
                      SHA1_CTX* context)
{
  u32 i;
  unsigned char finalcount[8];
  ....
  memset(context->count, 0, 8);
  memset(finalcount, 0, 8);
}

Предупреждение PVS-Studio: V597 The compiler could delete the 'memset' function call, which is used to flush 'finalcount' buffer. The memset_s() function should be used to erase the private data. wifi_generate_pin.c 185

Software weaknesses type — CWE-14: Compiler Removal of Code to Clear Buffers

Компилятор вправе удалить функцию memset, которая затирает приватные данные в буфере finalcount. С точки зрения языка C и C++, вызов функции можно удалить, так как далее этот буфер нигде не используется. Хочу обратить внимание, что это не теоретически возможное поведение компилятора, а самое что ни на есть обыденное. Компиляторы действительно удаляют такие функции (см. V597, CWE-14).

Путаница с выделением и освобождением памяти (2 ошибки)


Первая ошибка.
void
GenericTableContent::set_max_key_length (size_t max_key_length)
{
  ....
  std::vector *offsets;
  std::vector *offsets_attrs;

  offsets = new(std::nothrow)                       // <=
            std::vector  [max_key_length];
  if (!offsets) return;

  offsets_attrs = new(std::nothrow)
                  std::vector  [max_key_length];
  if (!offsets_attrs) {
    delete offsets;                                 // <=
    return;
  }
  ....
}

Предупреждение PVS-Studio: V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] offsets;'. scim_generic_table.cpp 998

Software weaknesses type — CWE-762: Mismatched Memory Management Routines

В переменной offset хранится указатель на массив объектов, созданных с помощью оператора new[]. Следовательно, разрушаться эти объекты должны с помощью оператора delete[].

Вторая ошибка.
static void __draw_remove_list(SettingRingtoneData *ad)
{
  char *full_path = NULL;
  ....
  full_path = (char *)alloca(PATH_MAX);                  // <=
  ....
  if (!select_all_item) {
    SETTING_TRACE_ERROR("select_all_item is NULL");
    free(full_path);                                     // <=
    return;
  }
  ....  
}

Предупреждение PVS-Studio: V611 The memory was allocated using 'alloca' function but was released using the 'free' function. Consider inspecting operation logics behind the 'full_path' variable. setting-ringtone-remove.c 88

Software weaknesses type — CWE-762: Mismatched Memory Management Routines

Память для буфера выделяется на стеке. Далее возможна ситуация, когда адрес этого буфера будет передан в качестве фактического аргумента в функцию free, что недопустимо.

Потенциально неинициализированная переменная (1 ошибка)


Тело функции _app_create, в которой находится ошибка, весьма длинное, поэтому я выделю только самую суть:
Eext_Circle_Surface *surface;
....
if (_WEARABLE)
  surface = eext_circle_surface_conformant_add(conform);
....
app_data->circle_surface = surface;

Преду
Нравится

Комментировать — К дневнику — Страницы: [1] [Новые]
 

Добавить комментарий:
Текст комментария: смайлики

Проверка орфографии: (найти ошибки)

Прикрепить картинку:

 Переводить URL в ссылку
 Подписаться на комментарии
 Подписать картинку


О проекте