Главная
Блог разработчиков phpBB
 
+ 17 предустановленных модов
+ SEO-оптимизация форума
+ авторизация через соц. сети
+ защита от спама

Повторная проверка TortoiseSVN с поддержкой анализатора кода PVS-Studio

Anna | 25.06.2014 | нет комментариев
Мы отправили разработчикам TortoiseSVN на некоторое время безвозмездный ключ для анализатора PVS-Studio. Пока они не поспели им воспользоваться, я решил стремительно скачать начальные коды TortoiseSVN и самосильно исполнить обзор. Цель внятна. Очередная маленькая статья для рекламы PVS-Studio.

Мы теснее проверяли план TortoiseSVN. Это было давным-давно. Проверка плана как раз совпала с выпуском PVS-Studio 4.00, где впервой возникли диагностические правила всеобщего назначения.Время от времени мы повторяем проверки, Дабы продемонстрировать пользу от регулярного применения инструмента. Нет смысла один либо два раза проверить план. В изменяемом коде непрерывно возникают новые ошибки. А потом медлительно и уныло исправляются. Соответственно, максимальная польза будет достигнута при ежедневном применении PVS-Studio. А ещё отменнее, при применении инкрементального обзора.

Выходит, посмотрим, что удалось обнаружить увлекательного в плане с поддержкой PVS-Studio версии 5.05. Начальные коды TortoiseSVN были взяты 19.06.2013 из http://tortoisesvn.googlecode.com/svn/trunk. Кстати, план TortoiseSVN является дюже добротным и имеет громадную базу пользователей-программистов. Так что если обнаружить хоть что-то, это теснее огромное достижение.

Необычные данные

static void ColouriseA68kDoc (....)
{
  if (((sc.state == SCE_A68K_NUMBER_DEC) && isdigit(sc.ch))
      ....
      || ((sc.state == SCE_A68K_MACRO_ARG) && isdigit(sc.ch))
      || ((sc.state == SCE_A68K_MACRO_ARG) && isdigit(sc.ch))
      ....
}

Диагностическое сообщение: V501 There are identical sub-expressions ‘((sc.state == 11) && isdigit(sc.ch))’ to the left and to the right of the ‘||’ operator. lexa68k.cxx 160

Присутствует два идентичных сопоставления. Допустимо, имеет место опечатка.

Опечатка, вероятно, присутствует и в дальнейшем коде. Два раза проверяется значение переменной ‘rv’.

struct hentry * AffixMgr::compound_check(
  ....
  if (rv && forceucase && (rv) && ....)
  ....
}

Диагностическое сообщение: V501 There are identical sub-expressions to the left and to the right of the ‘&&’ operator: rv && forceucase && (rv):

  • affixmgr.cxx 1784
  • affixmgr.cxx 1879

Дальнейший фрагмент кода:

bool IsAllASCII7 (const CString& s)
{
  for (int i = 0, count = s.GetLength(); i < count;   i)
    if (s[i] >= 0x80)
      return false;
    return true;
}

Диагностическое сообщение: V547 Expression ‘s[i] >= 0×80′ is always false. The value range of char type: [-128, 127]. logdlgfilter.cpp 34

Функция IsAllASCII7() неизменно возвращает ‘true’. Условие ‘s[i] >= 0×80′ неизменно ложно. Значение переменной типа ‘char’ не может быть огромнее либо равно 0×80.

Дальнейший фрагмент кода с неправильным сопоставлением:

int main(int argc, char **argv)
{
  ....
  DWORD ticks;
  ....
  if (run_timers(now, &next)) {
    ticks = next - GETTICKCOUNT();
    if (ticks < 0) ticks = 0;
  } else {
    ticks = INFINITE;
  }
  ....
}

Диагностическое сообщение: V547 Expression ‘ticks < 0′ is always false. Unsigned type value is never < 0. winplink.c 635

Переменная ‘ticks’ является беззнаковой. Это значит, что проверка «if (ticks < 0)» не имеет смысла. Обстановка происхождения переполнения не будет обработана.

Разглядим ошибку, из-за которой функция ‘strncmp’ сопоставляет строки не всецело.

int  AffixMgr::parse_convtable(...., const char * keyword)
{
  char * piece;
  ....
  if (strncmp(piece, keyword, sizeof(keyword)) != 0) {
  ....
}

Диагностическое сообщение: V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. affixmgr.cxx 3654

Оператор ‘sizeof’ вычисляет размер указателя. Это значение никак не связанно с длиной строки.

Подозрительное образование строки

Функции с переменным числом доводов неизменно всюду есть, и как неизменно опасны.

class CTSVNPath
{
  ....
private:
  mutable CString m_sBackslashPath;
  mutable CString m_sLongBackslashPath;
  mutable CString m_sFwdslashPath;
  ....
};

const FileStatusCacheEntry * SVNFolderStatus::BuildCache(
  const CTSVNPath& filepath, ....)
{
  ....
  CTraceToOutputDebugString::Instance() (_T(__FUNCTION__)
    _T(": building cache for %sn"), filepath);
  ....
}

Диагностическое сообщение: V510 The ‘operator()’ function is not expected to receive class-type variable as second actual argument:

  • svnfolderstatus.cpp 150
  • svnfolderstatus.cpp 355
  • svnfolderstatus.cpp 360

Спецификатор “%s” указывает, что в качестве фактического довода функция ждет строку. Впрочем, переменная ‘filepath’ совсем не строка, а трудный объект, состоящий из множества строк. Затрудняюсь сказать, что будет распечатано и не упадёт ли вообще данный код.

Небезопасно применять такие функции, как ‘printf()’ дальнейшим образом: «printf(myStr);». Если внутри ‘myStr’ будут присутствовать руководящие спецификаторы, то программа может распечатать то, что ей не полагается либо аварийно кончаться.

Разглядим код из TortoiseSVN:

BOOL CPOFile::ParseFile(....)
{
  ....
  printf(File.getloc().name().c_str());
  ....
}

Диагностическое сообщение: V618 It’s dangerous to call the ‘printf’ function in such a manner, as the line being passed could contain format specification. The example of the safe code: printf(“%s”, str); pofile.cpp 158

Если имя файла будет «myfile%s%i%s.txt», то итог будет печален.

Примечание. У нас есть увлекательная заметка на тему угрозы применения функции printf().

Неправильное обнуление массивов

Я не знаю, насколько для TortoiseSVN небезопасно оставить содержимое буферов, не обнулив их. Допустимо, вообще неопасно. Впрочем есть код для обнуления буферов. А от того что он не работает, стоит про это упомянуть. Ошибки выглядят так:

static void sha_mpint(SHA_State * s, Bignum b)
{
  unsigned char lenbuf[4];
  ....
  memset(lenbuf, 0, sizeof(lenbuf));
}

Диагностическое сообщение: V597 The compiler could delete the ‘memset’ function call, which is used to flush ‘lenbuf’ buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sshdss.c 23

Перед выходом из функции, массив ‘lenbuf’ следует очистить. Так как после этого массив огромнее не применяется, оптимизатор удалит вызов функции ‘memset’. Дабы этого не происходило, требуется применять особые функции.

Другие места, где компилятор удалит вызов ‘memset()’:

  • sshdss.c 37
  • sshdss.c 587
  • sshdes.c 861
  • sshdes.c 874
  • sshdes.c 890
  • sshdes.c 906
  • sshmd5.c 252
  • sshrsa.c 113
  • sshpubk.c 153
  • sshpubk.c 361
  • sshpubk.c 1121
  • sshsha.c 256

Необычное

BOOL InitInstance(HINSTANCE hResource, int nCmdShow)
{
  ....
  app.hwndTT; // handle to the ToolTip control
  ....
}

Диагностическое сообщение: V607 Ownerless expression ‘app.hwndTT’. tortoiseblame.cpp 1782

Скорее каждого, в функции ‘InitInstance()’, член ‘hwndTT’ должен чем-то инициализироваться. Впрочем, из-за опечатки, код оказался недописанным.

64-битные ошибки

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

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

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

void LoginDialog::CreateModule(void)
{
  ....
  DialogBoxParam(g_hmodThisDll, MAKEINTRESOURCE(IDD_LOGIN),
                 g_hwndMain, (DLGPROC)(LoginDialogProc),
                 (long)this);
  ....
}

Диагностическо/> Разглядим иной пример везения. В программе TortoiseSVN тип HWND зачастую применяется как тип ‘unsigned’. Для этого доводится выполняться очевидные реформирования типа. Скажем, так, как показано в следующих функциях:

HWND m_hWnd;
UINT_PTR uId;
INT_PTR CBaseView::OnToolHitTest(....) const
{
  ....
  pTI->uId = (UINT)m_hWnd;
  ....
}

UINT_PTR  idFrom;
HWND m_hWnd;

BOOL CBaseView::OnToolTipNotify(
  UINT, NMHDR *pNMHDR, LRESULT *pResult)
{
  if (pNMHDR->idFrom != (UINT)m_hWnd)
    return FALSE;
  ....
}

Либо, скажем, значение переменной типа HWND печатается, как если бы это был тип ‘long’.

bool CCommonAppUtils::RunTortoiseProc(....)
{
  ....
  CString sCmdLine;
  sCmdLine.Format(L"%s /hwnd:%ld",
    (LPCTSTR)sCommandLine, AfxGetMainWnd()->GetSafeHwnd());
  ....
}

Официально данный код неверен. Дело в том, что тип ‘HWND’ представляет собой указатель. А значит, его невозможно превращать в 32-битные целочисленные типы. И анализатор PVS-Studio переживает по этому поводу, выдавая предупреждения.

Но увлекательно то, что данный код будет трудиться абсолютно правильно!

Тип HWND применяется для хранения дескрипторов, которые применяются в Windows для работы с разными системными объектами. Такими же типами являются HANDLE, HMENU, HPALETTE, HBITMAP и так дальше.

Правда дескрипторы являются 64-битными указателями, для большей совместимости (скажем, для вероятности взаимодействия между 32-битынми и 64-битными процессами) в них применяется только младшие 32-бита. Подробнее смотри “Microsoft Interface Definition Language (MIDL): 64-Bit Porting Guide” (USER and GDI handles are sign extended 32b values).

Помещая тип HWND в 32-битные типы, разработчики вряд ли базировались именно на этих допущениях. Скорее каждого, это не дюже опрятный код, работающий правильно вследствие везению и стараниям разработчиков Windows API.

Итог

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

Ссылки

Добавочные ссылки, которые могут пояснить некоторые тонкие моменты, описываемые в статье.

  1. База познаний. Перезаписывать память — для чего?
  2. Документация. V668. There is no sense in testing the pointer against null, as the memory was allocated using the ‘new’ operator.
  3. База познаний. Как правильно привести указатель к int в 64-битной программе?
  4. Карпов Андрей, Евгений Рыжков. Уроки разработки 64-битных приложений на языке Си/Си .

Источник: programmingmaster.ru

Оставить комментарий
Форум phpBB, русская поддержка форума phpBB
Рейтинг@Mail.ru 2008 - 2017 © BB3x.ru - русская поддержка форума phpBB