Заходи
Гость

Хостинг

Статистика
Яндекс.Метрика Счетчик PR-CY.Rank
Онлайн всего: 1
Гостей: 1
Пользователей: 0

Ccылки

Свежак

Главная » Статьи » Все статьи » Прогараммирование

Статический анализ: ошибки в медиаплеере и безглючная аська
Продолжу экскурсию по ошибкам в программах и демонстрацию полезности статического анализа кода.

 Планирую, что через неделю вы уже сможете попробовать первую beta-версию с новым набором правил общего назначения.

Рассмотрим два проекта. Первый - Fennec Media Project. Это универсальный медиа-плеер ориентированный на воспроизведение аудио и видео в высоком разрешении. В комплект исходных кодов входит множество модулей расширения (plugins) и кодеков, но анализироваться будет только сам плеер. Исходный код последней на данный момент версии 1.2 Alpha доступен здесь.

Второй проект - qutIM. Это кроссплатформенный клиент мгновенного обмена сообщениями с открытым исходным кодом. Был проанализирован код на момент начала ноября 2010 года. Набор исходных кодов был предоставлен мне одним из разработчиков, но вы также можете скачать исходный код с официального сайта.

Fennec Media Project. Небольшой нормальный проект, содержащий нормальное количество ошибок. Вот первая ошибка. Или первые две ошибки, смотря как считать. В общем, в двух местах вместо переменной 'b' используется переменная 'a'.

int fennec_tag_item_compare(struct fennec_audiotag_item *a,
 struct fennec_audiotag_item *b)
{
 int v;
 if(a->tsize && a->tsize)
 v = abs(str_cmp(a->tdata, a->tdata));
 else
 v = 1;
 return v;
}

PVS-Studio указал на этот код, так как условие "a->tsize && a->tsize" явно подозрительно.

Диагностическое сообщение и местоположение ошибки в коде:

V501 There are identical sub-expressions to the left and to the right of the '&&' operator: a -> tsize && a -> tsize media library.c 1076

А теперь родное и милое сердцу каждого программиста - лишние точки с запятой. Вот первый фрагмент:

int settings_default(void)
{
 ...
 for(i=0; i<16; i++);
 for(j=0; j<32; j++)
 {
 settings.conversion.equalizer_bands.boost[i][j] = 0.0;
 settings.conversion.equalizer_bands.preamp[i] = 0.0;
 }
}

Сообщение PVS-Studio и местоположение коде:

V529 Odd semicolon ';' after 'for' operator. settings.c 483

Второй фрагмент:

int trans_rest(transcoder_settings *trans)
{
 ...
 for(i=0; i<16; i++);
 {
 trans->eq.eq.preamp[i] = 0.0;
 for(j=0; j<32; j++)
 {
 trans->eq.eq.boost[i][j] = 0.0;
 }
 }
}

Сообщение PVS-Studio и местоположение коде:

V529 Odd semicolon ';' after 'for' operator. settings.c 913

Есть еще третий и четвертый фрагмент с ';'. Но приводить здесь не буду. Все однотипно и неинтересно.

Дальше не совсем ошибка, но почти. Вместо функции _beginthreadex используется CreateThread. Вызовов CreateThread в Fennec несколько, но приведу только один пример:

t_sys_thread_handle sys_thread_call(t_sys_thread_function cfunc)
{
 unsigned long tpr = 0;
 unsigned long tid = 0;
 return (t_sys_thread_handle)
 CreateThread(0, 0, cfunc, &tpr, 0,&tid);
}

Предупреждение PVS-Studio и местоположение коде:

V513 Use _beginthreadex/_endthreadex functions instead of CreateThread/ExitThread functions. system.c 331

Вдаваться сейчас вглубь вопроса, почему следует использовать _beginthreadex/_endthreadex вместо CreateThread/ExitThread, не буду. Напишу совсем кратко, а подробные обсуждения данного вопроса можно почитать здесьздесь и здесь.

В священном писании (в MSDN) сказано:

A thread in an executable that calls the C run-time library (CRT) should use the _beginthreadex and _endthreadex functions for thread management rather than CreateThread and ExitThread; this requires the use of the multi-threaded version of the CRT. If a thread created using CreateThread calls the CRT, the CRT may terminate the process in low-memory conditions.

В общем лучше подстраховаться и всегда вызывать именно _beginthreadex/_endthreadex. Кстати именно так рекомендует поступать и Джеффри Рихтера в шестой главе "Windows для профессионалов: создание эффективных Win32-приложений с учетом специфики 64-разрядной версии Windows" / Пер. с англ. - 4-е изд.

Обнаружилось несколько неудачных использований функции memset. Кстати, я до недавнего времени думал, что беспокойство, связанное с использованием memset, memcmp, memcpy – дело прошлого. Мол, это раньше так писали, но сейчас все знают про опасности, аккуратны с этими функциями, используют sizeof(), используют контейнеры из STL и так далее. А сейчас все розовое и мягкое. Оказывается, что нет. Я за последний месяц столько ляпов с этими функциями насмотрелся. Так что все эти ошибки по-прежнему цветут и пахнут.

Вернемся в Fennec. Первый memset:

#define uinput_size 1024
typedef wchar_t letter;

letter uinput_text[uinput_size];

string basewindows_getuserinput(const string title,
 const string cap, const string dtxt)
{
 memset(uinput_text, 0, uinput_size);
 ...
}

Диагностика PVS-Studio и местоположение коде:

V512 A call of the 'memset' function will lead to a buffer overflow or underflow. base windows.c 151

На первый взгляд с "memset(uinput_text, 0, uinput_size);" все хорошо. И возможно даже и было хорошо, в те времена, когда тип 'letter' был 'char'. Но теперь это 'wchar_t' и как результат мы чистим только половину буфера.

Второй неудачный memset:

typedef wchar_t letter;
letter name[30];

int Conv_EqualizerProc(HWND hwnd,UINT uMsg,
 WPARAM wParam,LPARAM lParam)
{
 ...
 memset(eqp.name, 0, 30);
 ...
}

Воистину магические числа – это зло. Вроде и не сложно написать "sizeof(eqp.name)". Но упорно не пишем и продолжаем вновь и вновь отстреливать себе ногу :).

Диагностика PVS-Studio и местоположение коде:

V512 A call of the 'memset' function will lead to a buffer overflow or underflow. base windows.c 2892

Ну и еще в одном месте такая шибка есть:

V512 A call of the 'memset' function will lead to a buffer overflow or underflow. transcode settings.c 588

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

В Windows API есть структуры, в которых указатели на строки должны заканчиваться двойным нулем. Наиболее используемым является член lpstrFilter в структуре OPENFILENAME. Этот параметр на самом деле указывает на набор строк, разделенных символом '\0'. А для того чтобы узнать, что строки закончились и нужны два нуля в конце.

Вот только это очень просто забыть. Фрагмент кода:

int JoiningProc(HWND hwnd,UINT uMsg,
 WPARAM wParam,LPARAM lParam)
{
 ...
 OPENFILENAME lofn;
 memset(&lofn, 0, sizeof(lofn));
 ...
 lofn.lpstrFilter = uni("All Files (*.*)\0*.*");
 ...
}

Сообщение PVS-Studio и местоположение коде:

V540 Member 'lpstrFilter' should point to string terminated by two 0 characters. base windows.c 5309

Будет диалог работать нормально или нет, зависит от того, что будет расположено в памяти после строки "All Files (*.*)\0*.*". По правильному здесь следовало написать "All Files (*.*)\0*.*\0". Один ноль явно указали мы, еще один ноль добавит компилятор.

Аналогичная беда и с другими диалогами.

int callback_presets_dialog(HWND hwnd, UINT msg,
 WPARAM wParam, LPARAM lParam)
{
 ...
 // SAVE
 OPENFILENAME lofn;
 memset(&lofn, 0, sizeof(lofn));
 ...
 lofn.lpstrFilter = uni("Equalizer Preset (*.feq)\0*.feq");
 ...
 ...
 // LOAD
 ...
 lofn.lpstrFilter = uni("Equalizer Preset (*.feq)\0*.feq");
 ...
}
int localsf_show_save_playlist(void)
{
 OPENFILENAME lofn;
 memset(&lofn, 0, sizeof(lofn));
 ...
 lofn.lpstrFilter = uni("Text file (*.txt)\0*.txt\0M3U file\0*.m3u");
 ...
}

Диагностика PVS-Studio и местоположение в коде:

V540 Member 'lpstrFilter' should point to string terminated by two 0 characters. base windows.c 986

V540 Member 'lpstrFilter' should point to string terminated by two 0 characters. base windows.c 1039

V540 Member 'lpstrFilter' should point to string terminated by two 0 characters. shared functions.c 360

Теперь подозрительная функция. Очень подозрительная. Впрочем, действительно тут ошибка или просто неудачно написано, я не знаю:

unsigned long ml_cache_getcurrent_item(void)
{
 if(!mode_ml)
 return skin.shared->audio.output.playlist.getcurrentindex();
 else
 return skin.shared->audio.output.playlist.getcurrentindex();
}

Диагностика PVS-Studio и местоположение в коде:

V523 The 'then' statement is equivalent to the 'else' statement. media library window.c 430

Я не стал заниматься анализом разнообразный модулей расширений, идущих вместе с Fennec. Но там не меньше разных грустных мест. Приведу только пару примеров. Фрагмент кода из проекта Codec ACC.

void MP4RtpHintTrack::GetPayload(...)
{
 ...
 if (pSlash != NULL) {
 pSlash++;
 if (pSlash != '\0') {
 length = strlen(pRtpMap) - (pSlash - pRtpMap);
 *ppEncodingParams = (char *)MP4Calloc(length + 1);
 strncpy(*ppEncodingParams, pSlash, length);
 }
}

Как следует из диагностического сообщения PVS-Studio:

V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *pSlash != '\0'. rtphint.cpp 346

здесь забыли разыменовать указатель. Получается, что мы делаем бессмысленное сравнение указателя с 0. Должно было быть: "if (*pSlash != '\0')".

Фрагмент кода из проекта Decoder Mpeg Audio:

void* tag_write_setframe(char *tmem,
 const char *tid, const string dstr)
{
 ...
 if(lset)
 {
 fhead[11] = '\0';
 fhead[12] = '\0';
 fhead[13] = '\0';
 fhead[13] = '\0';
 }
 ...
}

Диагностическое сообщение PVS-Studio и местоположение в коде:

V525 The code containing the collection of similar blocks. Check items '11', '12', '13', '13' in lines 716, 717, 718, 719. id3 editor.c 716

Вот оно зло метода Copy-Paste :).

В целом на проекте Fennec Media Project анализ общего назначения в PVS-Studio показал себя очень хорошо. Анализ был выполнен с низким процентом ложных срабатываний. Всего PVS-Studio указал на 31 фрагмент кода. При этом в 19 местах код действительно следует поправить.

Теперь перейдем к проекту qitIM.

Вот с эти проектом PVS-Studio потерпел поражение. Несмотря на то, что проект достаточно крупный (около 200 тысяч сток), анализатор PVS-Studio не смог выявить в нем ошибок. Хотя они конечно есть. Они везде и всегда есть :). И разработчики qitIM с этим не спорят, так как в некоторых случаях qitIM умудряется падать.

Приходится засчитать одно очко "команде ошибок".

Что это означает? Это означает:

1) Проект qitIM очень качественный проект. И хотя он тоже содержит ошибки, но они весьма редки и слишком высокого уровня для статического анализа (по крайней мере, для PVS-Studio).

2) PVS-Studio предстоит еще долгий путь развития и обучения более высокоуровневым диагностикам. Теперь стало более очевидно к чему стремиться. Цель - найти в qitIM хотя бы парочку настоящих ошибок.

Выдал ли что-то PVS-Studio для проекта qitIM? Выдал. Но немного и почти все ложные срабатывания. Их представляющего хоть какой-то интерес, можно выделить только следующее.

A) Используются функции CreateThread.

B) Найдено несколько странных функций. Потом один из авторов qitIM сообщил, что это забытые заглушки. Странность в том, что одна имеет имя save(), другая cancel(), но их содержимое идентично:

void XSettingsWindow::save()
{
 QWidget *c = p->stackedWidget->currentWidget();
 while (p->modifiedWidgets.count()) {
 SettingsWidget *widget = p->modifiedWidgets.takeFirst();
 widget->save();
 if (widget != c)
 widget->deleteLater();
 }
 p->buttonBox->close();
}

void XSettingsWindow::cancel()
{
 QWidget *c = p->stackedWidget->currentWidget(); 
 while (p->modifiedWidgets.count()) {
 SettingsWidget *widget = p->modifiedWidgets.takeFirst();
 widget->save();
 if (widget != c)
 widget->deleteLater();
 } 
 p->buttonBox->close();
}

Диагностика PVS-Studio:

V524 It is odd that the 'cancel' function is fully equivalent to the 'save' function (xsettingswindow.cpp, line 256). xsettingswindow.cpp 268

Надеюсь было интересно, и вы скоро захотите попробовать PVS-Studio 4.00 Beta. Конечно, PVS-Studio пока находит мало ошибок общего вида, но ведь это только начало. При этом исправление даже одной единственной ошибки на этапе кодирования может сэкономить массу нервов заказчикам, тестерам и программистам.

Категория: Прогараммирование | Добавил: Iron (06.06.2012)
Просмотров: 534 | Рейтинг: 0.0/0
Всего комментариев: 0
Добавлять комментарии могут только зарегистрированные пользователи.
[ Регистрация | Вход ]
Поиск

Статьи
[Менеджмент]
Факторы микросреды организации
[Операционные системы]
Диаграмма перехода процесса
[Операционные системы]
Особенности применения технологий Lotus Domino и Notes в современных информационных системах
[Менеджмент]
Факторы микросреды организации
[Прогараммирование]
ПОДПРОГРАММЫ. ПРОЦЕДУРЫ И ФУНКЦИИ
[Прогараммирование]
ДОКУМЕНТИРОВАНИЕ ПРОГРАММ
[Устронение ошибок систем]
Устранение неполадок при возникновении (синего экрана) Blue Screen Of Death (BSOD) (2)
[Прогараммирование]
Оптимизация программ Delphi
[Прогараммирование]
Как работает CSS?
[Прогараммирование]
Файлы

Категории
Операционные системы [30]
Устронение ошибок систем [13]
Безопасность систем [9]
Прогараммирование [32]
Технологические [0]
Информатика [23]
Бухгалтерский учет [3]
Ценообразование [0]
Экономика [0]
Менеджмент [3]
Психология [0]
Разное [4]

Популярный софт
Iron Kaspersky Internet Security 2015
Kaspersky Internet Security 2015
Iron Virtual DJ
Virtual DJ
Iron SoundForge 11
SoundForge 11
Iron Alcohol 120
Alcohol 120
Iron Norton Internet Security 2014
Norton Internet Security 2014
Iron Loaris Trojan Remover
Loaris Trojan Remover

Жми

Copyright MyCorp © 2019Конструктор сайтов - uCoz