CppCat провереяет OpenMW: у всесвіті Morrowind не все гладко

    CppCat провереяет OpenMW
Я перевірив проект OpenMW за допомогою CppCat і написав цю крихітну статтю. Знайшлося занадто мало помилок. Але мене просили написати про перевірку цього проекту статтю, і ось вона.
 
 OpenMW
OpenMW — це спроба відтворити популярну RPG Morrowind, повноцінна реалізація усіх особливостей гри з відкритим вихідним кодом. Для запуску OpenMW знадобиться оригінальний диск Morrowind.
 
Вихідний код доступний за адресою: https://code.google.com/p/openmw/
 
 CppCat
CppCat — молодший брат статичного аналізатора PVS-Studio. Про відмінності: " Порівняння можливостей статичних аналізаторів коду PVS-Studio і CppCat ".
 
 Знайдені підозрілі місця
 Частковий N1
 
std::string getUtf8(unsigned char c,
  ToUTF8::Utf8Encoder& encoder, ToUTF8::FromType encoding)
{
  ....
  conv[0xa2] = 0xf3;
  conv[0xa3] = 0xbf;
  conv[0xa4] = 0x0;
  conv[0xe1] = 0x8c;
  conv[0xe1] = 0x8c;   <<<<====
  conv[0xe3] = 0x0;
  ....
}

Попередження CppCat: V519 The 'conv [0xe1]' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 103, 104. Openmw fontloader.cpp 104
 
Напевно, це помилка. У виділеному рядку, швидше за все повинен використовуватися індекс 0xe2.
 
 Частковий N2
 
enum Flags
{
  ....
  NoDuration = 0x4,
  ....
}

bool CastSpell::cast (const ESM::Ingredient* ingredient)
{
  ....
  if (!magicEffect->mData.mFlags & ESM::MagicEffect::NoDuration)
  ....
}

Попередження CppCat: V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. openmw spellcasting.cpp 717
 
Помилка пов'язана з пріоритетом операцій. На початку, виконується дія (! MagicEffect-> mData.mFlags). Результат: 0 або 1. Потім виконується дія: 0 & 4 або 1 & 4. Це не має ніякого практичного сенсу. Швидше за все, код повинен виглядати так:
 
if ( ! (magicEffect->mData.mFlags & ESM::MagicEffect::NoDuration) )

 Частковий N3
 
void Clothing::blank()
{
  mData.mType = 0;
  mData.mWeight = 0;
  mData.mValue = 0;
  mData.mEnchant = 0;
  mParts.mParts.clear();
  mName.clear();
  mModel.clear();
  mIcon.clear();
  mIcon.clear();
  mEnchant.clear();
  mScript.clear();
}

Попередження CppCat: V586 The 'clear' function is called twice for deallocation of the same resource. Check lines: 48, 49. Components loadclot.cpp 49
 
Два рази очищається об'єкт 'mIcon'. Друга очищення зайва або треба було очистити щось ще.
 
 Частковий N4
 
void Storage::loadDataFromStream(
  ContainerType& container, std::istream& stream)
{
  std::string line;
  while (!stream.eof())
  {
    std::getline( stream, line );
    ....
  }
  ....
}

Попередження CppCat: V663 Infinite loop is possible. The 'cin.eof ()' condition is insufficient to break from the loop. Consider adding the 'cin.fail ()' function call to the conditional expression. components translation.cpp 45
 
При роботі з класом 'std :: istream' недостатньо виклику функції 'eof ()' для завершення циклу. У разі виникнення збою при читанні даних, виклик функції 'eof ()' буде завжди повертати значення 'false'. Для завершення циклу в цьому випадку необхідна додаткова перевірка значення, що повертається функцією 'fail ()'.
 
 Частковий N5
 
class Factory
{
  ....
  bool getReadSourceCache() { return mReadSourceCache; }
  bool getWriteSourceCache() { return mReadSourceCache; }
  ....
  bool mReadSourceCache;
  bool mWriteSourceCache;
  ....
};

Попередження CppCat: V524 It is odd that the body of 'getWriteSourceCache' function is fully equivalent to the body of 'getReadSourceCache' function. components factory.hpp 209
 
Мені здається, функція getWriteSourceCache () повинна бути такою:
 
bool getWriteSourceCache() { return mWriteSourceCache; }

 Частковий N6, N7, N8
 
std::string rangeTypeLabel(int idx)
{
  const char* rangeTypeLabels [] = {
    "Self",
    "Touch",
    "Target"
  };
  if (idx >= 0 && idx <= 3)
    return rangeTypeLabels[idx];
  else
    return "Invalid";
}

Попередження CppCat: V557 Array overrun is possible. The value of 'idx' index could reach 3. Esmtool labels.cpp 502
 
Неправильна перевірка індексу масиву. Якщо змінна 'idx' буде дорівнює 3, то відбудеться вихід за кордон масиву.
 
Правильно:
 
if (idx >= 0 && idx < 3)

Аналогічна ситуація зустрілася ще в двох місцях:
     
  • V557 Array overrun is possible. The value of 'idx' index could reach 143. Esmtool labels.cpp 391
  •  
  • V557 Array overrun is possible. The value of 'idx' index could reach 27. Esmtool labels.cpp 475
  •  
 Частковий N9
 
enum UpperBodyCharacterState
{
  UpperCharState_Nothing,
  UpperCharState_EquipingWeap,
  UpperCharState_UnEquipingWeap,
  ....
};

bool CharacterController::updateWeaponState()
{
  ....
  if((weaptype != WeapType_None ||
      UpperCharState_UnEquipingWeap) && animPlaying)
  ....
}

Попередження CppCat: V560 A part of conditional expression is always true: UpperCharState_UnEquipingWeap. openmw character.cpp 949
 
Ця умова дуже підозріле. Зараз його можна спростити до: «if (animPlaying)». Явно щось не так.
 
 Частковий N10, N11
 
void World::clear()
{
  mLocalScripts.clear();
  mPlayer->clear();
  ....
  if (mPlayer)
  ....
}

Попередження CppCat: V595 The 'mPlayer' pointer was utilized before it was verified against nullptr. Check lines: 234, 245. Openmw worldimp.cpp 234
 
Аналогічно: V595 The 'mBody' pointer was utilized before it was verified against nullptr. Check lines: 95, 99. Openmw physic.cpp 95
 
 Частковий N12
 
void ExprParser::replaceBinaryOperands()
{
  ....
  if (t1==t2)
    mOperands.push_back (t1);
  else if (t1=='f' || t2=='f')
    mOperands.push_back ('f');
  else
    std::logic_error ("failed to determine result operand type");
}

Попередження CppCat: V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw logic_error (FOO); components exprparser.cpp 101
 
Забули ключове слово 'throw'. Код має бути таким:
 
throw std::logic_error ("failed to determine result operand type");

 Висновок
Придбайте кожному Сі + + програмісту у вашій команді по CppCat, і ви заощадите масу часу на усунення помилок і найрізноманітніших помилок.
 
При покупці CppCat для команди розробників, надаються знижки.
 
 цю статтю англійською
Якщо хочете поділитися цією статтею з англомовною аудиторією, то прошу використовувати посилання на переклад: Andrey Karpov. CppCat Checks OpenMW: Not All is Fine in the Morrowind Universe .
 
 Прочитали статтю і є питання? Часто до наших статей ставлять одні і ті ж питання. Відповіді на них ми зібрали тут: Відповіді на питання читачів статей про PVS-Studio і CppCat, версія 2014 . Будь ласка, ознайомтеся зі списком.
 
    
Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.