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.

>
>
>
Checking Appleseed source code

Checking Appleseed source code

Sep 29 2015

Majority of the projects we report about in the articles contain dozens of PVS-Studio analyzer warnings. Of course we choose just a small portion of data from the analyzer report to be in our articles. There are some projects though, where the quantity of warnings is not that high and the number of some interesting "bloomers" is just not enough for an article. Usually these are small projects, which ceased developing. Today I'm going to tell you about Appleseed project check, the code of which we found quite high-quality, from the point of view of the analyzer.

0349_Appleseed/image1.png

Introduction:

Appleseed is a modern, open source, physically-based rendering engine designed to produce photorealistic images, animations and visual effects. It provides individuals and small studios with an efficient, reliable suite of tools built on robust foundations and open technologies.

This project contains 700 source code files. Our PVS-Studio analyzer found just several warnings of 1-st and 2-nd level that could be of interest to us.

Check results

V670 The uninitialized class member 'm_s0_cache' is used to initialize the 'm_s1_element_swapper' member. Remember that members are initialized in the order of their declarations inside a class. animatecamera cache.h 1009

class DualStageCache
  : public NonCopyable
{
  ....
    S1ElementSwapper    m_s1_element_swapper;     // <=Line 679
    S1Cache             m_s1_cache;

    S0ElementSwapper    m_s0_element_swapper;
    S0Cache             m_s0_cache;               // <=Line 683
};

FOUNDATION_DSCACHE_TEMPLATE_DEF(APPLESEED_EMPTY)
DualStageCache(
    KeyHasherType&      key_hasher,
    ElementSwapperType& element_swapper,
    const KeyType&      invalid_key,
    AllocatorType       allocator)
  : m_s1_element_swapper(m_s0_cache, element_swapper)//warning...
  // warning: referring to an uninitialized member
  , m_s1_cache(m_s1_element_swapper, allocator)
  , m_s0_element_swapper(m_s1_cache)
  , m_s0_cache(key_hasher, m_s0_element_swapper, invalid_key)
{
}

The analyzer found a possible error in the constructor class initialization. Judging by the comment: "warning: referring to an uninitialized member", which has already been in the code, we see that the developers know that for the 'm_s1_element_swapper' field initialization another uninitialized 'm_s0_cache' filed may be used. They are not correcting it though. According to the language standard, the order of initialization of the class members in the constructor goes in their declaration order in the class.

V605 Consider verifying the expression: m_variation_aov_index < ~0. An unsigned value is compared to the number -1. appleseed adaptivepixelrenderer.cpp 154

size_t m_variation_aov_index;
size_t m_samples_aov_index;

virtual void on_tile_end(
                         const Frame& frame,
                         Tile& tile,
                         TileStack& aov_tiles) APPLESEED_OVERRIDE
{
  ....
  if (m_variation_aov_index < ~0)                           // <=
    aov_tiles.set_pixel(x, y, m_variation_aov_index, ....);

  if (m_samples_aov_index != ~0)                            // <=
    aov_tiles.set_pixel(x, y, m_samples_aov_index, ....);
  ....
}

The inversion result of '~0' is -1, having the int type. Then this number converts into an unsigned size_t type. It's not crucial, but not really graceful. It is recommended to specify a SIZE_MAX constant in such expression right away.

At first glance there is no evident error here. But my attention was drawn by the usage of two different conditional operators, though both conditions check the same. The conditions are true if the variables are not equal to the maximum possible size_t type value (SIZE_MAX). These checks are differently written. Such a code looks very suspicious; perhaps there can be some logical error here.

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

char* duplicate_string(const char* s)
{
    assert(s);

    char* result = new char[strlen(s) + 1];

    if (result)
        strcpy(result, s);

    return result;
}

The analyzer detected a situation when the pointer value, returned by the 'new' operator, is compared to null. We should remember, that if the 'new' operator could not allocate the memory, then according to the C++ language standard, an exception std::bad_alloc() would be generated.

Thus in the Appleseed project, which is compiled into Visual Studio 2013, the pointer comparison with null will be meaningless. And one day such function usage can lead to an unexpected result. It is assumed that the duplicate_string() function will return nullptr if it can't create a string duplicate. It will generate an exception instead, that other parts of the program may be not ready for.

V719 The switch statement does not cover all values of the 'InputFormat' enum: InputFormatEntity. appleseed inputarray.cpp 92

enum InputFormat
{
    InputFormatScalar,
    InputFormatSpectralReflectance,
    InputFormatSpectralIlluminance,
    InputFormatSpectralReflectanceWithAlpha,
    InputFormatSpectralIlluminanceWithAlpha,
    InputFormatEntity
};

size_t add_size(size_t size) const
{
    switch (m_format)
    {
      case InputFormatScalar:
        ....
      case InputFormatSpectralReflectance:
      case InputFormatSpectralIlluminance:
        ....
      case InputFormatSpectralReflectanceWithAlpha:
      case InputFormatSpectralIlluminanceWithAlpha:
        ....
    }

    return size;
}

And where is the case for InputFormatEntity? This switch() block contains neither a default section, nor a variable action with the 'InputFormatEntity' value. Is it a real error or the author deliberately missed the value?

There are two more fragments (cases) like that:

  • V719 The switch statement does not cover all values of the 'InputFormat' enum: InputFormatEntity. appleseed inputarray.cpp 121
  • V719 The switch statement does not cover all values of the 'InputFormat' enum: InputFormatEntity. appleseed inputarray.cpp 182

If there is no 'default' section and handling of all variable values, you may possibly miss the code addition for a new 'InputFormat' value and not be aware of that for a very long time.

V205 Explicit conversion of pointer type to 32-bit integer type: (unsigned long int) strvalue appleseed snprintf.cpp 885

#define UINTPTR_T unsigned long int

int
portable_vsnprintf(char *str, size_t size, const char *format,
                                                    va_list args)
{
  const char *strvalue;
  ....
  fmtint(str, &len, size,
              (UINTPTR_T)strvalue, 16, width,               // <=
              precision, flags);
  ....
}

Finally we found quite a serious error that shows up in a 64-bit version of the program. Appleseed is a cross platform project that can be compiled on Windows and Linux. To get the project files we use Cmake. In the Windows compilation documentation it is suggested to use "Visual Studio 12 Win64" that's why except the general diagnostics (GA, General Analysis), I've also looked through the diagnostics of 64-bit errors (64, Viva64) of the PVS-Studio analyzer.

The full identification code of 'UINTPTR_T' macro looks like this:

/* Support for uintptr_t. */
#ifndef UINTPTR_T
#if HAVE_UINTPTR_T || defined(uintptr_t)
#define UINTPTR_T uintptr_t
#else
#define UINTPTR_T unsigned long int
#endif /* HAVE_UINTPTR_T || defined(uintptr_t) */
#endif /* !defined(UINTPTR_T) */

The uintptr_t is an unsigned, integer memsize-type that can safely hold a pointer no matter what the platform architecture is, although for Windows compilation was defined "unsigned long int" type. The type size depends on the data model, and unlike Linux OS, the 'long' type is always 32-bits in Windows. That's why the pointer won't fit into this variable type on Win64 platform.

Conclusion

All in all the Appleseed project, which is quite a big one, contains only a few analyzer's warnings. That's why it proudly gets a medal "Clear Code" and can no longer be afraid of our unicorn.

0349_Appleseed/image2.png
Popular related articles
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…
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…
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…
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…
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…
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…
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…
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…
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…

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