Андрей Карпов

13 Окт 2014

Теги:

#Cpp #Knowledge #Compiler

Тяп-ляп, проверил библиотеки Visual C++ 2013 (update 3)

13 Окт 2014

Автор: Андрей Карпов

Результат проверки
Мелочи
Заключение

Мне предложили проверить библиотеки, входящие в Visual Studio 2013. Ничего особенно примечательного я не нашёл. Только несколько мелких ошибок и недочётов. Интригующую статью из этого не сделаешь, но я всё равно опишу замеченные недостатки. Надеюсь, это сделает библиотеки чуть лучше, и подвигнет авторов провести более тщательную проверку. У меня нет файлов проектов для сборки библиотек. Поэтому я проверял файлы кое-как, и много могло быть пропущено.

0288_Visual_C++_2013_(update3)_ru/image1.png

Это уже вторая статья о проверке библиотек, входящих в Visual C++. О результатах предыдущей проверки можно узнать из статьи: Обнаружены ошибки в библиотеках Visual C++ 2012.

Проверить полноценно библиотеки я не могу. Я поступил очень топорно. Включил в новый проект все файлы, содержащиеся в папках "crt\src" и "atlmfc\src". Плюс я сделал новый test.cpp файл, куда включил все заголовочные файлы, относящиеся к стандартной библиотеке (vector, map, set и так далее).

После этого, немного поколдовав с настройками проекта, я добился, что компилируется около 80% файлов. Думаю, этого достаточно. Даже если файл не компилируется, он, скорее всего, будет проверен PVS-Studio, пусть и не полностью.

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

Для проверки я использовал PVS-Studio версии 5.19. Проверялись исходные коды Си/Си++ библиотек, входящие в состав Visual Studio 2013 (update 3).

Результат проверки

Встретились некоторые недочёты, которые мы находили и в предыдущей версии Visual Studio 2012. Например, всё так же странно выглядит функция proj(), опасно устроен деструктор ~single_link_registry(). Но повторяться не интересно. Попробуем найти что-то новое.

Неправильная проверка индекса

void _Initialize_order_node(...., size_t _Index, ....)
{
  if (_Index < 0)
  {
    throw std::invalid_argument("_Index");
  }
  ....
}

Предупреждение PVS-Studio: V547 Expression '_Index < 0' is always false. Unsigned type value is never < 0. agents.h 8442

Аргумент _Index и имеет беззнаковый тип. Поэтому проверка не имеет смысла. Исключение никогда не будет сгенерировано. Думаю, это не ошибка, а просто лишний код.

Ошибка формата

int _tfpecode; /* float point exception code */

void __cdecl _print_tiddata1 (
  _ptiddata ptd
)
{
  ....
  printf("\t_gmtimebuf      = %p\n",   ptd->_gmtimebuf);
  printf("\t_initaddr       = %p\n",   ptd->_initaddr);
  printf("\t_initarg        = %p\n",   ptd->_initarg);
  printf("\t_pxcptacttab    = %p\n",   ptd->_pxcptacttab);
  printf("\t_tpxcptinfoptrs = %p\n",   ptd->_tpxcptinfoptrs);
  printf("\t_tfpecode       = %p\n\n", ptd->_tfpecode);
  ....
}

Предупреждение PVS-Studio: V576 Incorrect format. Consider checking the second actual argument of the 'printf' function. The pointer is expected as an argument. tidprint.c 133

Здесь мы имеем дело с эффектом последней строки. В конце блока однотипных строк допущена ошибка. Везде следует распечатывать значение указателя. Но в самом конце переменная '_tfpecode' является не указателем, а просто целочисленным значением. Должно быть написано:

printf("\t_tfpecode       = %i\n\n", ptd->_tfpecode);

Странные повторяющиеся вычисления

unsigned int SchedulerProxy::AdjustAllocationIncrease(....) const
{
  ....
  unsigned int remainingConcurrency = 
                         m_maxConcurrency - m_currentConcurrency;
  remainingConcurrency = m_maxConcurrency - m_currentConcurrency;
  ....
}

Предупреждение PVS-Studio: V519 The 'remainingConcurrency' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1136, 1137. schedulerproxy.cpp 1137

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

Подозрение на опечатку

double HillClimbing::CalculateThroughputSlope(....)
{
  ....
  MeasuredHistory * lastHistory = GetHistory(fromSetting);
  MeasuredHistory * currentHistory = GetHistory(toSetting);
  ....
  double varianceOfcurrentHistory = currentHistory->VarianceMean();
  double varianceOflastHistory = currentHistory->VarianceMean();
  ....
}

Предупреждение PVS-Studio: V656 Variables 'varianceOfcurrentHistory', 'varianceOflastHistory' are initialized through the call to the same function. It's probably an error or un-optimized code. Consider inspecting the 'currentHistory->VarianceMean()' expression. Check lines: 412, 413. hillclimbing.cpp 413

Подозрительно, что переменной varianceOfcurrentHistory и varianceOflastHistory присваивается одно и то же значение. Логично было-бы инициализировать varianceOflastHistory вот так:

double varianceOflastHistory = varianceOfcurrentHistory;

Более того, существует ещё указатель 'lastHistory'. Выскажу предположение, что код содержит опечатку. Возможно, код должен быть таким:

double varianceOfcurrentHistory = currentHistory->VarianceMean();
double varianceOflastHistory = lastHistory->VarianceMean();

А здесь точно опечатка

BOOL CPropertySheet::SetActivePage(CPropertyPage* pPage)
{
  ASSERT_VALID(this);
  ENSURE_VALID(pPage);
  ASSERT_KINDOF(CPropertyPage, pPage);

  int nPage = GetPageIndex(pPage);
  ASSERT(pPage >= 0);

  return SetActivePage(nPage);
}

Предупреждение PVS-Studio: V503 This is a nonsensical comparison: pointer >= 0. dlgprop.cpp 1206

Странно проверять, что значение указателя больше или равно нулю. Это опечатка и, на самом деле, хотели проверить переменную 'nPage':

int nPage = GetPageIndex(pPage);
ASSERT(nPage >= 0);

Конечно, это всего лишь ASSERT, и ошибка не приведёт ни к каким негативным последствиям. Но всё равно это ошибка.

Одни и те же действия, независимо от условия

void CMFCVisualManager::OnDrawTasksGroupCaption(....)
{
  ....
  if (pGroup->m_bIsSpecial)
  {
    if (!pGroup->m_bIsCollapsed)
    {
      CMenuImages::Draw(pDC, CMenuImages::IdArrowUp,
                        rectButton.TopLeft());
    }
    else
    {
      CMenuImages::Draw(pDC, CMenuImages::IdArrowDown,
                        rectButton.TopLeft());
    }
  }
  else
  {
    if (!pGroup->m_bIsCollapsed)
    {
      CMenuImages::Draw(pDC, CMenuImages::IdArrowUp,
                        rectButton.TopLeft());
    }
    else
    {
      CMenuImages::Draw(pDC, CMenuImages::IdArrowDown,
                        rectButton.TopLeft());
    }
  }
  ....
}

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

В независимости от условия (pGroup->m_bIsSpecial), выполняются одни и те же действия. Это странно.

Неправильная проверка номера порта

typedef WORD ATL_URL_PORT;
ATL_URL_PORT m_nPortNumber;

inline BOOL Parse(_In_z_ LPCTSTR lpszUrl)
{
  ....
  m_nPortNumber = (ATL_URL_PORT) _ttoi(tmpBuf);
  if (m_nPortNumber < 0)
    goto error;
  ....
}

Предупреждение PVS-Studio: V547 Expression 'm_nPortNumber < 0' is always false. Unsigned type value is never < 0. atlutil.h 2773

Переменная 'm_nPortNumber' имеет беззнаковый тип WORD.

Нет виртуального деструктора

class CDataSourceControl
{
  ....
  ~CDataSourceControl();
  ....
  virtual IUnknown* GetCursor();
  virtual void BindProp(....);
  virtual void BindProp(....);
  ....
}

CDataSourceControl* m_pDataSourceControl;

COleControlSite::~COleControlSite()
{
  ....
  delete m_pDataSourceControl;
  ....
}

Предупреждение PVS-Studio: V599 The destructor was not declared as a virtual one, although the 'CDataSourceControl' class contains virtual functions. occsite.cpp 77

Класс CDataSourceControl содержит в себе виртуальные методы, но при этом деструктор не является виртуальным. Это опасно. Если от класса CDataSourceControl будет унаследован класс X, то нельзя будет уничтожать объекты типа X, используя указатель на базовый класс.

Недописанный код

BOOL CMFCWindowsManagerDialog::OnHelpInfo(HELPINFO* pHelpInfo)
{
  pHelpInfo->iCtrlId;
  CWnd* pParentFrame = AfxGetMainWnd();
  pParentFrame->SendMessage(AFX_WM_WINDOW_HELP, 0,
                            (LPARAM) this);
  return FALSE;
}

Предупреждение PVS-Studio: V607 Ownerless expression 'pHelpInfo->iCtrlId'. afxwindowsmanagerdialog.cpp 472

Что такое "pHelpInfo->iCtrlId;"? Что бы это значило?

Подозрительная двойная инициализация

CMFCStatusBar::CMFCStatusBar()
{
  m_hFont = NULL;

  // setup correct margins
  m_cxRightBorder = m_cxDefaultGap;  // <=
  m_cxSizeBox = 0;

  m_cxLeftBorder = 4;
  m_cyTopBorder = 2;
  m_cyBottomBorder = 0;
  m_cxRightBorder = 0;               // <=
  ....
}

Предупреждение PVS-Studio: V519 The 'm_cxRightBorder' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 74, 80. afxstatusbar.cpp 80

В начале в переменную 'm_cxRightBorder' записывается значение другой переменной. А затем, то значение вдруг затирается нулём.

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

#define S_OK  ((HRESULT)0L)
#define E_NOINTERFACE  _HRESULT_TYPEDEF_(0x80004002L)

HRESULT GetDocument(IHTMLDocument2** ppDoc) const
{  
  const T* pT = static_cast<const T*>(this);
  return pT->GetDHtmlDocument(ppDoc) ? S_OK : E_NOINTERFACE;
}

HRESULT GetEvent(IHTMLEventObj **ppEventObj) const
{
  ....
  if (GetDocument(&sphtmlDoc))
  ....
}

Предупреждение PVS-Studio: V545 Such conditional expression of 'if' operator is incorrect for the HRESULT type value 'GetDocument(& sphtmlDoc)'. The SUCCEEDED or FAILED macro should be used instead. afxhtml.h 593

Оформление кода может не соответствовать его логике работы. Кажется, что, если условие 'GetDocument(...)' истинно, то удалось "получить" документ. Но, на самом деле, всё не так. Функция GetDocument() возвращает значение типа HRESULT. С этим типом всё наоборот. Например, статус S_OK закодирован как 0, а статус E_NOINTERFACE как 0x80004002L. Для проверки значений типа HRESULT следует использовать специальные макросы: SUCCEEDED, FAILED.

Я не знаю, есть здесь ошибка или нет, но этот код сбивает с толку, и его следует проверить.

Неправильный аргумент для макроса MAKE_HRESULT

#define MAKE_HRESULT(sev,fac,code) \
  ((HRESULT) \
   (((unsigned long)(sev)<<31) | \
    ((unsigned long)(fac)<<16) | \
    ((unsigned long)(code))) )

ATLINLINE ATLAPI AtlSetErrorInfo(....)
{
  ....
  hRes = MAKE_HRESULT(3, FACILITY_ITF, nID);
  ....
}

Предупреждение PVS-Studio: V673 The '(unsigned long)(3) << 31' expression evaluates to 6442450944. 33 bits are required to store the value, but the expression evaluates to the 'unsigned' type which can only hold '32' bits. atlcom.h 6650

Всё будет работать правильно, но ошибка есть. Поясню.

Функция должна сформировать информацию об ошибке в переменной типа HRESULT. Для этого используется макрос MAKE_HRESULT. Но используется он неправильно. Программист посчитал, что первый параметр 'severity' может лежать в приделах от 0 до 3. Видимо, он перепутал это со способом формирования кодов ошибки, используемые при работе с функциями GetLastError()/SetLastError().

Макрос MAKE_HRESULT в качестве первого аргумента может принимать только 0 (success) или 1 (failure). Подробнее этот вопрос рассматривался на форуме сайта CodeGuru: Warning! MAKE_HRESULT macro doesn't work.

Так как в качестве первого фактического аргумента используется число 3, то возникает переполнение. Число 3 "превратится" в 1. Благодаря этой случайности ошибка не повлияет на работу программы.

ASSERT-ы, условия которых всегда истинны

Встречается достаточно много ситуаций, когда условие для ASSERT выглядит так: (X >= 0). При этом, переменная X объявлена, как беззнаковый целочисленный тип. Получается, что условие всегда истинно.

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

Рассмотрим пример:

DWORD m_oversubscribeCount; 

void ExternalContextBase::Oversubscribe(....)
{
  if (beginOversubscription)
  {
    ASSERT(m_oversubscribeCount >= 0);
    ++m_oversubscribeCount;
  }
  ....
}

Предупреждение PVS-Studio: V547 Expression 'm_oversubscribeCount >= 0' is always true. Unsigned type value is always >= 0. externalcontextbase.cpp 204

Остальные предупреждения приведу просто списком:

V547 Expression 'm_oversubscribeCount >= 0' is always true. Unsigned type value is always >= 0. internalcontextbase.cpp 506
V547 Expression 'pGlobalNode->m_idleCores >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 3764
V547 Expression 'pGlobalNode->m_availableCores >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 3769
V547 Expression 'pReceivingProxyData->m_allocation >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4100
V547 Expression 'pReceivingProxyData->m_allocation >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4360
V547 Expression 'exclusiveCoresAvailable >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4657
V547 Expression 'coresNeeded >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4657
V547 Expression 'previousGlobal >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4781
V547 Expression 'currentGlobal >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4782
V547 Expression 'm_minConcurrency >= 0' is always true. Unsigned type value is always >= 0. schedulerproxy.cpp 63
V547 Expression 'm_minimumHardwareThreads >= 0' is always true. Unsigned type value is always >= 0. schedulerproxy.cpp 125
V547 Expression 'm_oversubscribeCount >= 0' is always true. Unsigned type value is always >= 0. umsthreadinternalcontext.cpp 308
V547 Expression 'j >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 1922
V547 Expression 'pMaxNode->m_availableCores >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 2542
V547 Expression 'previousLocal >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4793
V547 Expression 'currentLocal >= 0' is always true. Unsigned type value is always >= 0. resourcemanager.cpp 4794
V547 Expression is always true. Unsigned type value is always >= 0. schedulerpolicybase.cpp 285
V547 Expression 'value >= 0' is always true. Unsigned type value is always >= 0. schedulerpolicybase.cpp 345

Лишние приведения типов

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

Рассмотрим первый пример:

size_t __cdecl strnlen(const char *str, size_t maxsize);
size_t __cdecl _mbstrnlen_l(const char *s,
                            size_t sizeInBytes,
                            _locale_t plocinfo)
{
  ....
  if ( _loc_update.GetLocaleT()->locinfo->mb_cur_max == 1 )
      /* handle single byte character sets */
      return (int)strnlen(s, sizeInBytes);
  ....
}

Предупреждение PVS-Studio: V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'strnlen(s, sizeInBytes)'. _mbslen_s.c 67

Функция strnlen() возвращает значение типа 'size_t'. Затем оно вдруг явно приводится к типу 'int'. После чего, значение неявно вновь будет расширено до типа size_t.

Этот код содержит потенциальную 64-битную ошибку. Если вдруг кто-то захочет в 64-битной программе посчитать с помощью функции _mbstrnlen_l() количество символов в очень длинной строке, то он получит неверный результат.

Мне кажется, это явное приведении осталось в коде случайно и его следует просто удалить.

Рассмотрим второй пример:

WINBASEAPI SIZE_T WINAPI GlobalSize (_In_ HGLOBAL hMem);

inline void __cdecl memcpy_s(
  _Out_writes_bytes_to_(_S1max,_N)  void *_S1,
  _In_ size_t _S1max,
  _In_reads_bytes_(_N) const void *_S2,
  _In_ size_t _N);

AFX_STATIC HGLOBAL AFXAPI _AfxCopyGlobalMemory(....)
{
  ULONG_PTR nSize = ::GlobalSize(hSource);
  ....
  Checked::memcpy_s(lpDest, (ULONG)::GlobalSize(hDest),
                    lpSource, (ULONG)nSize);
  ....
}

Предупреждение PVS-Studio: V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'nSize'. olemisc.cpp 684.

Функция GlobalSize() возвращает тип SIZE_T. Аргументы функции memcpy_s() тоже имеют тип size_t.

Так зачем тогда сделано явное приведение типа "(ULONG)::GlobalSize(hDest)" ?

Если мы будем работать с буфером более 4Gb, то функция memcpy_s() будет копировать только часть массива.

Есть ещё несколько подозрительных приведений типов:

V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'wcslen(* vp ++)'. cenvarg.c 236
V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: '::GlobalSize(m_hGlobalMemory)'. fileshrd.cpp 48
V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'wcslen(lpsz)'. dumpcont.cpp 31
V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'wcslen(lpsz)'. dumpcont.cpp 82
V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: '(cElems * sizeof (CLSID))'. ctlcore.cpp 1975
V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'wParam'. afxtoolbarslistcheckbox.cpp 94
V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'nChars * sizeof (TCHAR)'. statreg.h 270

Использование до проверки

CMFCPopupMenu* CMFCCustomizeButton::CreatePopupMenu()
{
  ....
  if (m_pWndParentToolbar->IsLocked())
  {
    pMenu->GetMenuBar()->m_pRelatedToolbar = m_pWndParentToolbar;
  }

  pMenu->m_bRightAlign = m_bMenuRightAlign &&
    (m_pWndParentToolbar->GetExStyle() & WS_EX_LAYOUTRTL) == 0;

  BOOL bIsLocked = (m_pWndParentToolbar == NULL ||
                    m_pWndParentToolbar->IsLocked());
  ....
}

Предупреждение PVS-Studio: V595 The 'm_pWndParentToolbar' pointer was utilized before it was verified against nullptr. Check lines: 192, 199. afxcustomizebutton.cpp 192

В начале указатель 'm_pWndParentToolbar' разыменовывается в выражении 'm_pWndParentToolbar->IsLocked()'. А затем проверяется на равенство нулю: 'm_pWndParentToolbar == NULL'.

Такой код опасен и, думаю, не стоит объяснять почему.

Ещё один такой случай:

void COleControlSite::BindDefaultProperty(....)
{
  ....
  if (pDSCWnd != NULL)
  {
    ....
    m_pDSCSite = pDSCWnd->m_pCtrlSite;
    ....
    m_pDSCSite->m_pDataSourceControl->BindProp(this, TRUE);
    if (m_pDSCSite != NULL)
      m_pDSCSite->m_pDataSourceControl->BindColumns();
  }
  ....
}

Предупреждение PVS-Studio: V595 The 'm_pDSCSite' pointer was utilized before it was verified against nullptr. Check lines: 1528, 1529. occsite.cpp 1528

Лишние переменные

Лишние переменные - не ошибка. Но лишние, они и есть лишние, и от них стоит избавиться. Пример:

int GetImageCount() const
{
  CRect rectImage(m_Params.m_rectImage);
  if (m_Bitmap.GetCount() == 1)
  {
    HBITMAP hBmp = m_Bitmap.GetImageWell();
    BITMAP bmp;

    if (::GetObject(hBmp, sizeof(BITMAP), &bmp) ==
        sizeof(BITMAP))
    {
      return bmp.bmHeight / m_Params.m_rectImage.Height();
    }

    return 0;
  }

  return m_Bitmap.GetCount();
}

Предупреждение PVS-Studio: V808 'rectImage' object of 'CRect' type was created but was not utilized. afxcontrolrenderer.h 89

Прямоугольник 'rectImage' создаётся, но никак потом не используется. Лишняя строчка в программе и несколько лишних тактов при работе Debug версии.

Привожу список найденных ненужных переменных: vs2003_V808.txt

Мелочи

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

Перечислю некоторые фрагменты, которые можно улучшить.

Опасные сравнения с TRUE

_PHNDLR __cdecl signal(int signum, _PHNDLR sigact)
{
  ....
  if ( SetConsoleCtrlHandler(ctrlevent_capture, TRUE)
       == TRUE )
  ....
}

Предупреждение PVS-Studio: V676 It is incorrect to compare the variable of BOOL type with TRUE. winsig.c 255

Везде, в том числе и в MSDN говорится, что нехорошо сравнивать что-то с TRUE. Функция может вернуть любое значение, отличное от 0, и это будет считаться истинной. TRUE, это 1. Правильно всегда сравнивать: Foo() != FALSE.

Аналогичные проверки можно встретить здесь:

V676 It is incorrect to compare the variable of BOOL type with TRUE. event.cpp 448
V676 It is incorrect to compare the variable of BOOL type with TRUE. Correct expression is: 'retVal != FALSE'. resourcemanager.cpp 1437
V676 It is incorrect to compare the variable of BOOL type with TRUE. Correct expression is: 'retVal != FALSE'. resourcemanager.cpp 5027

Инкремент

void _To_array(
  ::Concurrency::details::_Dynamic_array<_EType>& _Array)
{
  _LockHolder _Lock(_M_lock);
  _M_iteratorCount++;

  for(_LinkRegistry::iterator _Link = _M_links.begin();
      *_Link != NULL; _Link++)
  {
    _Array._Push_back(*_Link);
  }
}

Предупреждение PVS-Studio: V803 Decreased performance. In case '_Link' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. agents.h 1713

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

Примечание. Заметки на эту тему:

Если авторы библиотек посчитают, что стоит позаниматься с инкрементом, то вот список мест для этого: vs2003_V803.txt.

Неправильное восстановление уровня предупреждений

#pragma warning (disable : 4311)
SetClassLongPtr(m_hWnd,
  GCLP_HBRBACKGROUND,
  PtrToLong(reinterpret_cast<void*>(
    ::GetSysColorBrush(COLOR_BTNFACE))));
#pragma warning (default : 4311)

Предупреждение V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 165, 167. afxbasepane.cpp 167

Правильным способом возвращения предыдущего состояние предупреждения является использование "#pragma warning(push[ ,n ])" и "#pragma warning(pop)".

Остальные места: vs2003_V665.txt.

Проверка (this == NULL)

Классика жанра:

_AFXWIN_INLINE CWnd::operator HWND() const
  { return this == NULL ? NULL : m_hWnd; }

Предупреждение PVS-Studio: V704 'this == 0' expression should be avoided - this expression is always false on newer compilers, because 'this' pointer can never be NULL. afxwin2.inl 19

К сожалению, это очень частый паттерн. Особенно в MFC. Но стоит потихоньку отучать людей использовать такие конструкции и показывать хороший пример.

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

Я понимаю, что поправить operator HWND() нет никакой возможности. Обратная совместимость важней. Но вдруг где-то это можно будет сделать безболезненно. Список таких проверок: vs2003_V704.txt

Заключение

Как видите получилась достаточно большая статья. Но, на самом деле, ничего существенного не найдено. Код библиотек Visual C++ однозначно качественный и отлаженный.

Буду рад, если эта статья поможет в дальнейшем сделать библиотеки Visual C++ чуть лучше. Отмечу ещё раз, что проверка была выполнена неполноценно. Разработчики библиотек Visual C++ смогут сделать это более качественно, так как имеют скрипты/проекты для сборки библиотек. Если будут какие-то сложности, я готов помочь - пишите нам в поддержку.

#Cpp #Knowledge #Compiler

Теги:

#Cpp #Knowledge #Compiler

Тяп-ляп, проверил библиотеки Visual C++ 2013 (update 3)

Результат проверки

Неправильная проверка индекса

Ошибка формата

Странные повторяющиеся вычисления

Подозрение на опечатку

А здесь точно опечатка

Одни и те же действия, независимо от условия

Неправильная проверка номера порта

Нет виртуального деструктора

Недописанный код

Подозрительная двойная инициализация

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

Неправильный аргумент для макроса MAKE_HRESULT

ASSERT-ы, условия которых всегда истинны

Лишние приведения типов

Использование до проверки

Лишние переменные

Мелочи

Опасные сравнения с TRUE

Инкремент

Неправильное восстановление уровня предупреждений

Проверка (this == NULL)

Заключение

Комментарии (0)

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Тип желаемой лицензии:

Хотите получить ключ на использование анализатора в пробный период?

Достижения

PVS-Studio

Лицензирование

Компания