To get a trial key
fill out the form below
Team License (standard version)
Enterprise License (extended version)
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Analysis of the The Powder Toy Simulator

Analysis of the The Powder Toy Simulator

Dec 24 2014

The Powder Toy is a free physics sandbox game, which simulates air pressure and velocity, heat, gravity and a countless number of interactions between different substances. The game provides you with various building materials, liquids, gases and electronic components which can be used to construct complex machines, guns, bombs, realistic terrains and almost anything else. You can browse and play thousands of different saves made by the community or upload your own. However, not everything is that good in the game: for a small project of about 350 files, it triggers too many warnings from our static analyzer. In this article, I'm going to show you the most interesting issues found in the project.

0298_The_Powder_Toy/image1.png

The Powder Toy was checked by PVS-Studio 5.20. The project is built under Windows in msys with the help of a Python script - that's why I had to use a special utility PVS-Studio Standalone to do the check. To learn more about the standalone version, see the article: PVS-Studio Now Supports Any Build System under Windows and Any Compiler. Easy and Right Out of the Box.

Analysis results

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

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

At a certain condition, a series of three items of an array of characters are to be checked, but because of a typo, item s[3] can't be checked, which is probably the reason for the program's incorrect behavior in certain situations.

V523 The 'then' statement is equivalent to the 'else' statement. button.cpp 142

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);
  ....
}

This is a function fragment with suspiciously similar code blocks. The conditional expression contains a series of logical operations, so I assume that it's not that this code fragment contains a pointless check, but there is a typo in the next to last function parameter 'draw_icon()'. That is, a value other than 255 should be written somewhere.

Similar fragments:

  • 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 The return value of function 'empty' is required to be utilized. requestbroker.cpp 309

std::vector<Request*> Children;

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

Instead of clearing the vector, the programmer called the 'empty()' function that doesn't change it. Since the code is inside a destructor, this error doesn't seem to affect program execution in any way. But I still thought this issue to be worth mentioning.

V547 Expression 'partsData[i] >= 256' is always false. The value range of unsigned char type: [0, 255]. gamesave.cpp 816

#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....
  ....
}

This code contains a suspicious piece only its author can conceive. Earlier, if the i-th item of the 'partsData' array was larger than or equal to 161, the value 28 was used to be written into the item. Now, the constant 161 is commented out and replaced with 256, which causes the condition to never be true as the maximum value of 'unsigned char' is 255.

V547 Expression is always false. Probably the '||' operator should be used here. previewview.cpp 449

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;
  }
  ....
}

Thanks to pure luck, part of the condition is always true. It is very probable that we are dealing with a typo here: perhaps it was the '||' operator that should have been used instead of '&&', or 'savePreview->Height' should be checked in one of the cases, for example.

V560 A part of conditional expression is always true: 0x00002. frzw.cpp 34

unsigned int Properties;

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

Everywhere in the code, bit operations are performed over the 'Properties' variable but in two places '||' is used instead of '|'. It means that 1 will be written into Properties there.

Here's another issue of this kind:

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

V567 Undefined behavior. The 'sandcolour_frame' variable is modified while being used twice between sequence points. simulation.cpp 4744

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

The 'sandcolour_frame ' variable is used twice in one sequence point. It results in an unpredictable result of such an expression. To learn more, see the description of the V567 diagnostic.

V570 The 'parts[i].dcolour' variable is assigned to itself. fwrk.cpp 82

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;              // <=
  ....
}

Suspicious initialization of a field to its own value.

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

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

To print a pointer, the %p specifier should be used.

V595 The 'gameSave' pointer was utilized before it was verified against nullptr. Check lines: 1063, 1070. gamecontroller.cpp 1063

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.");
  }
  ....
}

It would be more logical to check the 'gameSave' pointer for being null first and only then fill the fields.

A few other similar issues:

  • 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 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

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;          // <=
  }
  ....
}

Operators new, new[], delete, and delete[] should be used in corresponding pairs, i.e. a correct way to write this code is as follows: "delete[] userSession;".

It's not the only issue of this kind in the project:

  • 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 Uninitialized pointer 'ndata' used. simulation.cpp 1688

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

Until the intended modification of this fragment is carried out, the function will keep returning an uninitialized pointer.

Another similar place:

  • V614 Potentially uninitialized pointer 'tempThumb' used. saverenderer.cpp 150

Conclusion

The Powder Toy is an interesting cross-platform project that can be used for game, education and experiments. Despite its small size, I found it interesting to look into it. I hope the authors will find time to carry out analysis of the source code and study the complete analysis log.

Using static analysis regularly will help you save plenty of time to solve more serious tasks and TODO's.

Popular related articles
PVS-Studio ROI

Date: Jan 30 2019

Author: Andrey Karpov

Occasionally, we're asked a question, what monetary value the company will receive from using PVS-Studio. We decided to draw up a response in the form of an article and provide tables, which will sho…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: Oct 22 2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
Static analysis as part of the development process in Unreal Engine

Date: Jun 27 2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…
Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities

Date: Nov 21 2018

Author: Andrey Karpov

A brief description of technologies used in the PVS-Studio tool, which let us effectively detect a large number of error patterns and potential vulnerabilities. The article describes the implementati…
The Last Line Effect

Date: May 31 2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…
Appreciate Static Code Analysis!

Date: Oct 16 2017

Author: Andrey Karpov

I am really astonished by the capabilities of static code analysis even though I am one of the developers of PVS-Studio analyzer myself. The tool surprised me the other day as it turned out to be sma…
Free PVS-Studio for those who develops open source projects

Date: Dec 22 2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…
The Ultimate Question of Programming, Refactoring, and Everything

Date: Apr 14 2016

Author: Andrey Karpov

Yes, you've guessed correctly - the answer is "42". In this article you will find 42 recommendations about coding in C++ that can help a programmer avoid a lot of errors, save time and effort. The au…
Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives

Date: Jul 31 2017

Author: Andrey Karpov

After I wrote quite a big article about the analysis of the Tizen OS code, I received a large number of questions concerning the percentage of false positives and the density of errors (how many erro…
The way static analyzers fight against false positives, and why they do it

Date: Mar 20 2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…

Comments (0)

Next comments

This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept