Проверка проекта Ultimate Toolbox

В ходе тестирования анализатора общего назначения, входящего в PVS-Studio 4.00 было проверено несколько открытых проектов с сайта CodeProject. Одним из таких проектов стал Ultimate ToolBox.

В коде проекта Ultimate Toolbox был найден ряд ошибок, описание которых приведено ниже. Будет указано диагностическое сообщение, выданное анализатором, файл и номер строки. Также будет приведен фрагмент кода, содержащий ошибку и краткое описание ошибки. Более полно разобраться с примерами вам помогут приведенные в тексте ссылки.

1. Ошибка в условии

V501 There are identical sub-expressions to the left and to the right of the '&&' operator. UT ox3dtabview.cpp 230

void COX3DTabViewContainer::OnNcPaint() 
{
  ...
  if(rectClient.top<rectClient.bottom &&
     rectClient.top<rectClient.bottom)
  {
    dc.ExcludeClipRect(rectClient);
  }
  ...
}

Предупреждение V501 указывает на ошибку в условии. Скорее всего, после оператора '&&' должно следовать условие, сравнивающее left и right.

Аналогичная ошибка имеется так же здесь:

V501 There are identical sub-expressions to the left and to the right of the '&&' operator. UT oxtabclientwnd.cpp 184

2. Условие, которое всегда истинно.

V547 Expression 'lpDrawItemStruct -> itemID >= 0' is always true. Unsigned type value is always >= 0. UT oxautolistbox.cpp 56

void COXAutoListBox::DrawItem(...) 
{
  ...
  if (lpDrawItemStruct->itemID>=0)
  {
    ...
  }
  ...
}

Условие "lpDrawItemStruct->itemID>=0" всегда выполняется, так как член itemID имеет тип UINT. Подробнее аналогичные ошибки описаны в документации (V547). Скорее всего, здесь должен быть следующий код:

if (lpDrawItemStruct->itemID != (UINT)(-1))
{
  ...
}

3. Условие, которое всегда ложно.

V547 Expression 'lpms -> itemID < 0' is always false. Unsigned type value is never < 0. UT oxcoolcombobox.cpp 476

void COXCoolComboBox::MeasureItem(...)
{
  if(lpms->itemID<0)
    lpms->itemHeight=m_nDefaultFontHeight+1; 
  else
    lpms->itemHeight=
      m_nDefaultFontHeightSansLeading+1; 
}

Предупреждение V547 указывает на то, что всегда будет выполняться ветка "lpms->itemHeight=m_nDefaultFontHeight+1; ". Как и в предыдущем случае, это связано с тем, что член itemID имеет беззнаковый тип UINT.

4. Неэффективный код

V801 Decreased performance. It is better to redefine the first function argument as a reference. Consider replacing 'const .. mi' with 'const .. &mi'. UT oxdllmanager.h 123

BOOL operator==(const _tagMODULEINFO mi) const 
{
 return ((hModule==mi.hModule)&
   (sModuleFileName.CompareNoCase(mi.sModuleFileName)==0));
}

Этот код не содержит ошибки, но его можно сделать более эффективным. Нет необходимости создавать копию структуры _tagMODULEINFO каждый раз при вызове оператора '=='. Сообщение V801 указывает на то, что "const _tagMODULEINFO mi" можно заменить на "const _tagMODULEINFO &mi".

5. Ошибка в условии

V501 There are identical sub-expressions to the left and to the right of the '==' operator: dwDockStyle == dwDockStyle UT oxframewnddock.cpp 190

void COXFrameWndSizeDock::TileDockedBars(
  DWORD dwDockStyle)
{
  ...
  if (pDock != NULL &&
      (pDock->m_dwStyle &&
       dwDockStyle == dwDockStyle))
  ...
}

Видимо вместо выражения "dwDockStyle == dwDockStyle" должно быть написано что-то другое.

6. Работа с 'char' как с 'unsigned char'

Для одной строчки выдано сразу два предупреждения:

V547 Expression 'chNewChar >= 128' is always false. The value range of signed char type: [-128, 127]. UT oxmaskededit.cpp 81

V547 Expression 'chNewChar <= 255' is always true. The value range of signed char type: [-128, 127]. UT oxmaskededit.cpp 81

BOOL CMaskData::IsValidInput(TCHAR chNewChar)
{
   ...
  if((chNewChar >= 128) && (chNewChar <= 255))
    bIsValidInput=TRUE ;
  ...
}

Данное условие не имеет смысла, так как переменная chNewChar имеет диапазон значений [-128..127]. Это значит, что условие никогда не выполнится.

7. Ошибка в логике

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. UT oxprocess.h 583

inline COXProcessIterator& operator+(int nOffset)
{
  if(nOffset>0)
    Next(nOffset);
  else if(nOffset>0)
    Prev(-nOffset);
  return *this; 
}

Предупреждение V517 указывает на ошибку в логике программы. Ветка "Prev(-nOffset);" никогда не будет выполнена. Корректный код по всей видимости должен выглядеть так:

inline COXProcessIterator& operator+(int nOffset)
{
  if(nOffset>0)
    Next(nOffset);
  else if(nOffset<0)
    Prev(-nOffset);
  return *this; 
}

Аналогичные ошибки есть и в других местах программы:

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. UT oxprocess.h 596

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. UT oxprocess.h 610

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. UT oxprocess.h 624

8. Условие, которое всегда ложно.

V547 Expression 'm_nCurrentIndex - nOffset < 0' is always false. Unsigned type value is never < 0. UT oxprocess.cpp 594

int m_nCurrentIndex;
...
BOOL COXProcessIterator::Prev(UINT nOffset)
{
  ...
  if(m_nCurrentIndex-nOffset<0)
    return FALSE;
  ...
}

Так как выражение "m_nCurrentIndex-nOffset" имеет тип unsigned, то они никогда не может быть меньше 0.

9. Ошибка в ASSERT()

V501 There are identical sub-expressions to the left and to the right of the '&&' operator. UT oxscrollwnd.cpp 645

void COXScrollWnd::OnPrepareDC(...)
{
  ...
  ASSERT(m_totalDev.cx>=0 && m_totalDev.cx>=0);
  ...
}

Должно быть:

ASSERT(m_totalDev.cx>=0 && m_totalDev.cy>=0);

Аналогичная ошибка ещё имеется здесь:

V501 There are identical sub-expressions to the left and to the right of the '&&' operator. UT oxzoomvw.cpp 179