Мы постоянно проверяем открытые проекты на языке Си/Си++. Но почти всегда, это проекты, разрабатываемые в Visual Studio. А вот Embarcadero C++ Builder мы как-то обделили вниманием. Нужно исправляться и сегодня мы проверили проект WinSCP.
Поддержка C++ Builder была прекращена в PVS-Studio после версии 5.20. По всем возникшим вопросам вы можете обратиться в нашу поддержку.
WinSCP – свободный графический клиент протоколов SFTP и SCP, предназначенный для Windows. Распространяется по лицензии GNU GPL. Обеспечивает защищённое копирование файлов между компьютером и серверами, поддерживающими эти протоколы.
Официальный сайт: http://winscp.net
Для сборки проекта необходим Embarcadero C++ Builder XE2.
Проверка осуществлялась с помощью анализатора PVS-Studio. На данный момент PVS-Studio поддерживает:
Плюс вы можете запустить PVS-Studio Standalone. Он позволяет проверять заранее подготовленные *.i файлы или отслеживать процесс сборки проекта и собирать всю необходимую для проверки информацию. Подробности можно узнать здесь: "PVS-Studio теперь поддерживает любую сборочную систему".
Ошибок нашлось не много, но достаточно, чтобы написать эту статью и привлечь внимание пользователей Embarcadero RAD Studio.
TForm * __fastcall TMessageForm::Create(....)
{
....
LOGFONT AFont;
....
memset(&AFont, sizeof(AFont), 0);
....
}Предупреждение PVS-Studio: V575 The 'memset' function processes '0' elements. Inspect the third argument. messagedlg.cpp 786
Функция memset() принимает размер массива в третьем аргументе. Простая, но неприятная опечатка. Структура остаётся неинициализированной.
Ниже в коде имеется ещё одна идентичная опечатка: messagedlg.cpp 796
void __fastcall TCustomScpExplorerForm::EditorAutoConfig()
{
....
else
{
....
TEditorList EditorList;
EditorList = *WinConfiguration->EditorList;
EditorList.Insert(0, new TEditorPreferences(EditorData));
WinConfiguration->EditorList = &EditorList;
}
....
}Предупреждение PVS-Studio: V506 Pointer to local variable 'EditorList' is stored outside the scope of this variable. Such a pointer will become invalid. customscpexplorer.cpp 2633
Объект 'EditorList' будет уничтожен сразу после выхода из области видимости. Однако, в программе сохраняют указатель на этот объект и затем используют. Это приводит к неопределённому поведению.
bool __fastcall RecursiveDeleteFile(....)
{
SHFILEOPSTRUCT Data;
memset(&Data, 0, sizeof(Data));
....
Data.pTo = L"";
....
}Предупреждение PVS-Studio: V540 Member 'pTo' should point to string terminated by two 0 characters. common.cpp 1659
Обратите внимание на следующую строку в описании параметра pTo в MSDN: "Note This string must be double-null terminated".
Из-за ошибки в диалоге для работы с файлом будет содержаться мусор. Или не будет. Всё зависит от везения. Но код в любом случае неправилен.
int CFileZillaApi::Init(....)
{
....
m_pMainThread->m_hOwnerWnd=m_hOwnerWnd;
m_pMainThread->m_hOwnerWnd=m_hOwnerWnd;
....
}Предупреждение PVS-Studio: V519 The 'm_pMainThread->m_hOwnerWnd' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 88, 89. filezillaapi.cpp 89
Возможно, здесь нет никакой ошибки. Просто одна строка лишняя.
STDMETHODIMP CShellExtClassFactory::CreateInstance(....)
{
....
CShellExt* ShellExt = new CShellExt();
if (NULL == ShellExt)
{
return E_OUTOFMEMORY;
}
....
}Предупреждение PVS-Studio: V668 There is no sense in testing the 'ShellExt' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dragext.cpp 554
Проверка "if (NULL == ShellExt)" не имеет смысла, так как если не удастся выделить память, то оператор 'new' сгенерирует исключение std::bad_alloc.
bool CAsyncSslSocketLayer::CreateSslCertificate(....)
{
....
char buffer[1001];
int len;
while ((len = pBIO_read(bio, buffer, 1000)) > 0)
{
buffer[len] = 0;
fprintf(file, buffer);
}
....
}V618 It's dangerous to call the 'fprintf' function in such a manner, as the line being passed could contain format specification. The example of the safe code: printf("%s", str); asyncsslsocketlayer.cpp 2247
Если при записи в файл, в буфере будут содержаться управляющие спецификаторы, то результат окажется непредсказуемым. Безопасный способ:
fprintf(file, "%s", buffer);Вообще, эту ошибку можно считать потенциальной уязвимостью.
static error_t
client_send_propfind_request(....)
{
....
error_t err = 0;
int code = 0;
apr_hash_t * props = NULL;
const char * target = path_uri_encode(remote_path, pool);
char * url_path = apr_pstrdup(pool, target);
WEBDAV_ERR(neon_get_props(&props, ras, url_path,
NEON_DEPTH_ZERO, starting_props,
false, pool));
if (err && (err == WEBDAV_ERR_DAV_REQUEST_FAILED))
....
}Предупреждение: V560 A part of conditional expression is always false: (err == 1003). webdavfilesystem.cpp 10990
Где вы, программисты, использующие Embarcadero RAD Studio? Ау! Как показывает наша статистика, их почти нет. Приходите и попробуйте анализатор кода PVS-Studio!