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.

>
>
>
Celestia: Bugs' Adventures in Space

Celestia: Bugs' Adventures in Space

Oct 02 2019

Celestia is a three-dimensional space simulator. Simulation of the space allows exploring our universe in three dimensions. Celestia is available on Windows, Linux and macOS. The project is very small and PVS-Studio detected few defects in it. Despite this fact, we'd like to pay attention to it, as it's a popular educational project and it will be rather useful to somehow improve it. By the way, this program is used in popular films, series and programs for showing space. This fact, in turns, raises requirements to the code quality.

0673_Celestia/image1.png

Introduction

The official website of the Celestia project provides its detailed description. The source code is available on GitHub. The analyzer checked 166 .cpp files, excluding libraries and tests. The project is small, but found defects are noteworthy.

To do the source code analysis we used the PVS-Studio static code analyzer. Both Celestia and PVS-Studio are cross-platform. We analyzed the project on the Windows platform. It was simple to build the project by getting dependencies using Vcpkg - Microsoft library manager. According to reviews, it is inferior to Conan's capacities, but this program was also quite convenient to use.

Analysis results

Warning 1

V501 There are identical sub-expressions to the left and to the right of the '<' operator: b.nAttributes < b.nAttributes cmodfix.cpp 378

bool operator<(const Mesh::VertexDescription& a,
               const Mesh::VertexDescription& b)
{
  if (a.stride < b.stride)
    return true;
  if (b.stride < a.stride)
    return false;

  if (a.nAttributes < b.nAttributes)  // <=
    return true;
  if (b.nAttributes < b.nAttributes)  // <=
    return false;

  for (uint32_t i = 0; i < a.nAttributes; i++)
  {
    if (a.attributes[i] < b.attributes[i])
      return true;
    else if (b.attributes[i] < a.attributes[i])
      return false;
  }

  return false;
}

How easy it is to make a mistake when copying code. We write about it in every review. Apparently, only static code analysis can help out in this situation.

The programmer copied the conditional expression and didn't fully edit it. The correct version is most likely as follows:

if (a.nAttributes < b.nAttributes)
  return true;
if (b.nAttributes < a.nAttributes)
  return false;

An interesting research on this topic: "The Evil within the Comparison Functions".

Warning 2

V575 The 'memset' function processes '0' elements. Inspect the third argument. winmain.cpp 2235

static void BuildScriptsMenu(HMENU menuBar, const fs::path& scriptsDir)
{
  ....
  MENUITEMINFO info;
  memset(&info, sizeof(info), 0);
  info.cbSize = sizeof(info);
  info.fMask = MIIM_SUBMENU;
  ....
}

The code author mixed up the second and third arguments of the memset function. Instead of filling the structure with zeros, it says to fill 0 bytes of memory.

Warning 3

V595 The 'destinations' pointer was utilized before it was verified against nullptr. Check lines: 48, 50. wintourguide.cpp 48

BOOL APIENTRY TourGuideProc(....)
{
  ....
  const DestinationList* destinations = guide->appCore->getDestinations();
  Destination* dest = (*destinations)[0];
  guide->selectedDest = dest;
  if (hwnd != NULL && destinations != NULL)
  {
    ....
  }
  ....
}

The destinations pointer gets dereferenced two lines before it's compared with NULL. Such code can potentially lead to an error.

Warning 4

V702 Classes should always be derived from std::exception (and alike) as 'public' (no keyword was specified, so compiler defaults it to 'private'). fs.h 21

class filesystem_error : std::system_error
{
public:
  filesystem_error(std::error_code ec, const char* msg) :
    std::system_error(ec, msg)
  {
  }
}; // filesystem_error

The analyzer has detected the class inherited from the std::exception class via the private modifier (set by default). Such inheritance is dangerous because the std::exception exception will not be caught due to non-public inheritance. As a result, exception handlers behave not as intended.

Warning 5

V713 The pointer 's' was utilized in the logical expression before it was verified against nullptr in the same logical expression. winmain.cpp 3031

static char* skipUntilQuote(char* s)
{
  while (*s != '"' && s != '\0')
    s++;
  return s;
}

In one piece of the conditional expression the programmer forgot to dereference the s pointer. It turned out to be a comparison of the pointer, not its value. And it doesn't make sense in this situation.

Warning 6

V773 The function was exited without releasing the 'vertexShader' pointer. A memory leak is possible. modelviewwidget.cpp 1517

GLShaderProgram*
ModelViewWidget::createShader(const ShaderKey& shaderKey)
{
  ....
  auto* glShader = new GLShaderProgram();
  auto* vertexShader = new GLVertexShader();
  if (!vertexShader->compile(vertexShaderSource.toStdString()))
  {
      qWarning("Vertex shader error: %s", vertexShader->log().c_str());
      std::cerr << vertexShaderSource.toStdString() << std::endl;
      delete glShader;
      return nullptr;
  }
  ....
}

The memory is released by the glShader pointer but isn't cleared by the vertexShader pointer when exiting the function.

A similar fragment below:

  • V773 The function was exited without releasing the 'fragmentShader' pointer. A memory leak is possible. modelviewwidget.cpp 1526

Warning 7

V547 Expression '!inputFilename.empty()' is always true. makexindex.cpp 128

int main(int argc, char* argv[])
{
  if (!parseCommandLine(argc, argv) || inputFilename.empty())
  {
    Usage();
    return 1;
  }

  istream* inputFile = &cin;
  if (!inputFilename.empty())
  {
    inputFile = new ifstream(inputFilename, ios::in);
    if (!inputFile->good())
    {
      cerr << "Error opening input file " << inputFilename << '\n';
      return 1;
    }
  }
  ....
}

Repeated check of the file name presence. It's not a bug, but due to the fact that the inputFilename variable is already checked at the beginning of the function, the check below can be removed, making the code more compact.

Warning 8

V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: ... }. render.cpp 7457

enum LabelAlignment
{
  AlignCenter,
  AlignLeft,
  AlignRight
};

enum LabelVerticalAlignment
{
  VerticalAlignCenter,
  VerticalAlignBottom,
  VerticalAlignTop,
};

struct Annotation
{
  ....
  LabelVerticalAlignment valign : 3;
  ....
};

void Renderer::renderAnnotations(....)
{
  ....
  switch (annotations[i].valign)
  {
  case AlignCenter:
    vOffset = -font[fs]->getHeight() / 2;
    break;
  case VerticalAlignTop:
    vOffset = -font[fs]->getHeight();
    break;
  case VerticalAlignBottom:
    vOffset = 0;
    break;
  }
  ....
}

Enumeration values are mixed up in the switch operator. Because of this, enumerations of different types are compared in one fragment: LabelVerticalAlignment and AlignCenter.

Warning 9

V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 2844, 2850. shadermanager.cpp 2850

GLVertexShader*
ShaderManager::buildParticleVertexShader(const ShaderProperties& props)
{
  ....
  if (props.texUsage & ShaderProperties::PointSprite)
  {
    source << "uniform float pointScale;\n";
    source << "attribute float pointSize;\n";
  }

  if (props.texUsage & ShaderProperties::PointSprite)
  {
    source << DeclareVarying("pointFade", Shader_Float);
  }
  ....
}

The analyzer has detected two identical conditional expressions in a row. Either an error has been made or two conditions can be combined into one, and thus make the code simpler.

Warning 10

V668 There is no sense in testing the 'dp' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. windatepicker.cpp 625

static LRESULT
DatePickerCreate(HWND hwnd, CREATESTRUCT& cs)
{
  DatePicker* dp = new DatePicker(hwnd, cs);
  if (dp == NULL)
    return -1;
  ....
}

The value of the pointer returned by the new operator is compared with null. If the operator was unable to allocate memory, then according to the C++ standard, an exception std::bad_alloc() is thrown. Then the check for null is pointless.

Three more similar checks:

  • V668 There is no sense in testing the 'modes' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. winmain.cpp 2967
  • V668 There is no sense in testing the 'dropTarget' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. winmain.cpp 3272
  • V668 There is no sense in testing the 'appCore' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. winmain.cpp 3352

Warning 11

V624 The constant 3.14159265 is being utilized. The resulting value could be inaccurate. Consider using the M_PI constant from <math.h>. 3dstocmod.cpp 62

int main(int argc, char* argv[])
{
  ....
  Model* newModel = GenerateModelNormals(*model,
    float(smoothAngle * 3.14159265 / 180.0), weldVertices, weldTolerance);
  ....
}

The diagnostic is optional but in this case it's better to use the ready-made constant for the Pi number from the standard library.

Conclusion

Recently the project has been developed by enthusiasts, but is still popular and in demand in the training programs. There are thousands of addons with different space objects on the Internet. Celestia was used in the film "The Day After Tomorrow" and the documentary series "Through the Wormhole with Morgan Freeman".

We're glad that by checking various projects with open source code we're not only promoting static code analysis methodology, but also contribute to development of open source projects. By the way, you can also use the PVS-Studio analyzer not only to test your own, but also third-party projects as an enthusiast. To do this, you can use one of the options of free licensing.

Use static code analyzers, make your projects more reliable and better!

Popular related articles
The Evil within the Comparison Functions

Date: May 19 2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
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…
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…
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…
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…
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…
PVS-Studio for Java

Date: Jan 17 2019

Author: Andrey Karpov

In the seventh version of the PVS-Studio static analyzer, we added support of the Java language. It's time for a brief story of how we've started making support of the Java language, how far we've co…

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