# Проверка симулятора The Powder Toy

The Powder Toy является песочницей со свободной физикой, которая имитирует давление и скорость воздуха, тепла, тяжести и бесчисленное количество взаимодействий между различными веществами\. Игра предоставляет различные строительные материалы, жидкости, газы и электронные компоненты, которые могут быть использованы для построения сложных машин, оружия, бомб, реалистичной местности и почти всего, что угодно\. Вы можете просматривать и воспроизводить тысячи различных сделанных построек\. Вот только в игре оказалось не всё так замечательно: для небольшого проекта размером в \~350 файлов было получено довольно много предупреждений статического анализатора\. В этой статье будут описаны наиболее интересные места\. 

![0298_The_Powder_Toy_ru/image1.png](https://import.viva64.com/docx/blog/0298_The_Powder_Toy_ru/image1.png)

[The Powder Toy](https://powdertoy.co.uk/) проверялся с помощью PVS\-Studio 5\.20\. Проект собирается под Windows в msys с помощью скрипта на Python, поэтому для проверки я воспользовался специальной утилитой PVS\-Studio Standalone, которая описана в статье: [PVS\-Studio теперь поддерживает любую сборочную систему на Windows и любой компилятор\. Легко и "из коробки"](https://pvs-studio.ru/ru/blog/posts/0243/)\. 

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

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\|\|' operator: \!s\[1\] \|\|\!s\[2\] \|\|\!s\[1\] graphics\.cpp 829

```cpp
void Graphics::textsize(const char* s, int& width, int& height)
{
  ....
  else if (*s == '\x0F')
  {
    if(!s[1] || !s[2] || !s[1]) break;     // <=
    s+=3;                                  // <=
  }
  ....
}
```

При определённом условии выполняется проверка трёх последовательных элементов массива символов, но из\-за опечатки элемент s\[3\] не был проверен, что, возможно, приводит к неправильному поведению программы в некоторых ситуациях\.

[V523](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. button\.cpp 142

```cpp
void Button::Draw(const Point& screenPos)
{
  ....
  if(Enabled)
    if(isButtonDown || (isTogglable && toggle))
    {
      g->draw_icon(Position.X+iconPosition.X,
                   Position.Y+iconPosition.Y,
                   Appearance.icon, 255, iconInvert);
    }
    else
    {
      g->draw_icon(Position.X+iconPosition.X,
                   Position.Y+iconPosition.Y,
                   Appearance.icon, 255, iconInvert);
    }
  else
    g->draw_icon(Position.X+iconPosition.X,
                 Position.Y+iconPosition.Y,
                 Appearance.icon, 180, iconInvert);
  ....
}
```

Фрагмент функции с подозрительно одинаковым кодом\. Условное выражение содержит ряд логических операций, поэтому можно предположить, что данный фрагмент кода не содержит бесполезную проверку, а допущена опечатка в предпоследнем параметре функции 'draw\_icon\(\)'\. Т\.е\. где\-то должно быть написано не значение '255'\.

Похожие места:

* V523 The 'then' statement is equivalent to the 'else' statement\. luascriptinterface\.cpp 2758
* V523 The 'then' statement is equivalent to the 'else' statement\. searchview\.cpp 305

[V530](https://pvs-studio.ru/ru/docs/warnings/v530/) The return value of function 'empty' is required to be utilized\. requestbroker\.cpp 309

```cpp
std::vector<Request*> Children;

RequestBroker::Request::~Request()
{
  std::vector<Request*>::iterator iter = Children.begin();
  while(iter != Children.end())
  {
    delete (*iter);
    iter++;
  }
  Children.empty();             // <=
}
```

Вместо очистки вектора вызвали функцию 'empty\(\)', которая не изменяет вектор\. Так как код находится в деструкторе, то ошибка, пожалуй, никак не влияет на исполнение программы\. Но, тем не менее, решил озвучить этот момент\. 

[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'partsData\[i\] \>\= 256' is always false\. The value range of unsigned char type: \[0, 255\]\. gamesave\.cpp 816

```cpp
#define PT_DMND 28
//#define PT_NUM  161
#define PT_NUM 256

unsigned char *partsData = NULL,

void GameSave::readOPS(char * data, int dataLength)
{
  ....
  if(partsData[i] >= PT_NUM)
    partsData[i] = PT_DMND; //Replace all invalid elements....
  ....
}
```

Код содержит подозрительное место, которое понятно только автору\. Раньше, если i\-й элемент массива 'partsData' был больше или равен 161, то в элемент записывалось значение 28\. Сейчас же константа 161 закомментирована и заменена на 256, вследствие чего условие никогда не выполняется, т\.к\. максимальное значение 'unsigned char' – 255\.

[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression is always false\. Probably the '\|\|' operator should be used here\. previewview\.cpp 449

```cpp
void PreviewView::NotifySaveChanged(PreviewModel * sender)
{
  ....
  if(savePreview && savePreview->Buffer &&
     !(savePreview->Width == XRES/2 &&           // <=
       savePreview->Width == YRES/2))            // <=
  {
    pixel * oldData = savePreview->Buffer;
    float factorX = ((float)XRES/2)/((float)savePreview->Width);
    float factorY = ((float)YRES/2)/((float)savePreview->Height);
    float scaleFactor = factorY < factorX ? factorY : factorX;
    savePreview->Buffer = Graphics::resample_img(....);
    delete[] oldData;
    savePreview->Width *= scaleFactor;
    savePreview->Height *= scaleFactor;
  }
  ....
}
```

Благодаря везению часть условия всегда истинна\. С большой вероятностью тут имеет место опечатка: возможно, должен быть использован оператор '\|\|' вместо '&&', или в одном случае необходимо проверять, например, 'savePreview\-\>Height'\.

[V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: 0x00002\. frzw\.cpp 34

```cpp
unsigned int Properties;

Element_FRZW::Element_FRZW()
{
  ....
  Properties = TYPE_LIQUID||PROP_LIFE_DEC;
  ....
}
```

Во всём коде с переменной 'Properties' выполняются битовые операции, но в двух местах используется '\|\|' вместо '\|'\. Получается, что в Properties будет записано значение 1\.

Второе такое место:

* V560 A part of conditional expression is always true: 0x04000\. frzw\.cpp 34

[V567](https://pvs-studio.ru/ru/docs/warnings/v567/) Undefined behavior\. The 'sandcolour\_frame' variable is modified while being used twice between sequence points\. simulation\.cpp 4744

```cpp
void Simulation::update_particles()
{
  ....
  sandcolour_frame = (sandcolour_frame++)%360;
  ....
}
```

Переменная 'sandcolour\_frame ' дважды используется в одной точке следования\. В результате невозможно предсказать результат работы такого выражения\. Подробнее – в описании диагностики [V567](https://pvs-studio.ru/ru/docs/warnings/v567/)\.

[V570](https://pvs-studio.ru/ru/docs/warnings/v570/) The 'parts\[i\]\.dcolour' variable is assigned to itself\. fwrk\.cpp 82

```cpp
int Element_FWRK::update(UPDATE_FUNC_ARGS)
{
  ....
  parts[i].life=rand()%10+18;
  parts[i].ctype=0;
  parts[i].vx -= gx*multiplier;
  parts[i].vy -= gy*multiplier;
  parts[i].dcolour = parts[i].dcolour;              // <=
  ....
}
```

Подозрительная инициализация поля собственным значением\.

[V576](https://pvs-studio.ru/ru/docs/warnings/v576/) Incorrect format\. Consider checking the third actual argument of the 'printf' function\. To print the value of pointer the '%p' should be used\. powdertoysdl\.cpp 3247

```cpp
int SDLOpen()
{
  ....
  SDL_SysWMinfo SysInfo;
  SDL_VERSION(&SysInfo.version);
  if(SDL_GetWMInfo(&SysInfo) <= 0) {
      printf("%s : %d\n", SDL_GetError(), SysInfo.window);
      exit(-1);
  }
  ....
}
```

Для печати указателя необходимо использовать спецификатор %p\.

[V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'gameSave' pointer was utilized before it was verified against nullptr\. Check lines: 1063, 1070\. gamecontroller\.cpp 1063

```cpp
void GameController::OpenLocalSaveWindow(bool asCurrent)
{
  Simulation * sim = gameModel->GetSimulation();
  GameSave * gameSave = sim->Save();                        // <=
  gameSave->paused = gameModel->GetPaused();
  gameSave->gravityMode = sim->gravityMode;
  gameSave->airMode = sim->air->airMode;
  gameSave->legacyEnable = sim->legacy_enable;
  gameSave->waterEEnabled = sim->water_equal_test;
  gameSave->gravityEnable = sim->grav->ngrav_enable;
  gameSave->aheatEnable = sim->aheat_enable;
  if(!gameSave)                                             // <=
  {
    new ErrorMessage("Error", "Unable to build save.");
  }
  ....
}
```

Логичнее будет сначала проверить указатель 'gameSave' на ноль, а потом уже заполнять поля\.

Ещё несколько таких мест:

* V595 The 'newSave' pointer was utilized before it was verified against nullptr\. Check lines: 972, 973\. powdertoysdl\.cpp 972
* V595 The 'gameSave' pointer was utilized before it was verified against nullptr\. Check lines: 1271, 1278\. gamecontroller\.cpp 1271
* V595 The 'gameSave' pointer was utilized before it was verified against nullptr\. Check lines: 1323, 1330\. gamecontroller\.cpp 1323
* V595 The 'state\_' pointer was utilized before it was verified against nullptr\. Check lines: 220, 232\. engine\.cpp 220

[V611](https://pvs-studio.ru/ru/docs/warnings/v611/) The memory was allocated using 'new T\[\]' operator but was released using the 'delete' operator\. Consider inspecting this code\. It's probably better to use 'delete \[\] userSession;'\. apirequest\.cpp 106

```cpp
RequestBroker::ProcessResponse
APIRequest::Process(RequestBroker & rb)
{
  ....
  if(Client::Ref().GetAuthUser().ID)
  {
    User user = Client::Ref().GetAuthUser();
    char userName[12];
    char *userSession = new char[user.SessionID.length() + 1];
    ....
    delete userSession;          // <=
  }
  ....
}
```

Операторы new, new\[\], delete, delete\[\] должны использоваться соответствующими парами, т\.е\. здесь правильно будет: "delete\[\] userSession;"\.

И это место не единственное:

* V611 The memory was allocated using 'new T\[\]' operator but was released using the 'delete' operator\. Consider inspecting this code\. It's probably better to use 'delete \[\] userSession;'\. webrequest\.cpp 106
* V611 The memory was allocated using 'new T\[\]' operator but was released using the 'delete' operator\. Consider inspecting this code\. It's probably better to use 'delete \[\] workingDirectory;'\. optionsview\.cpp 228

[V614](https://pvs-studio.ru/ru/docs/warnings/v614/) Uninitialized pointer 'ndata' used\. simulation\.cpp 1688

```cpp
void *Simulation::transform_save(....)
{
  void *ndata;
  ....
  //ndata = build_save(....); //TODO: IMPLEMENT
  ....
  return ndata;
}
```

До запланированной доработки этого места функция будет возвращать неинициализированный указатель\.

Похожее место:

* V614 Potentially uninitialized pointer 'tempThumb' used\. saverenderer\.cpp 150

## Заключение

The Powder Toy \- интересный кроссплатформенный проект, который можно использовать для игры, обучения и экспериментов\. Несмотря на небольшой размер проекта, было интересно в нём разобраться\. Надеюсь, авторы уделят внимание анализу исходного кода и проанализируют полный лог проверки\.

Используя статический анализ регулярно, можно сэкономить массу времени на решение более полезных задач и TODO'шек\.