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 the Source Code of Nana Librar…

Checking the Source Code of Nana Library with PVS-Studio

Jul 13 2016

With the appearance of new C++ standards, C++ developers started to move to the new programming style, known as Modern C++, and projects that make use of the new style started to appear. Static code analyzers have to keep up to date to be able to detect errors in modern C++ code, which holds true for PVS-Studio as well. With Nana project as a test sample, we'll try to find out if PVS-Studio analyzer can cope with modern C++.

0411_NanaLibrary/image1.png

Introduction

To begin with, I'd like to say a few words about the project. Nana is a cross-platform C++11 library for creating graphical user interfaces. The library is small - 74 KLOC. It supports Windows and Linux (X11) platforms and provides experimental Mac OS support. Nana is an open-source software product distributed under the Boost Software License. We took version 1.3.0 for the check; its source code can be downloaded here: https://sourceforge.net/projects/nanapro/files/latest/download.

Typos in conditions

Typos in conditional statements are rather common, and Nana has a few of those too, for example:

V501 There are identical sub-expressions 'fgcolor.invisible()' to the left and to the right of the '&&' operator. text_editor.cpp 1316

void text_editor::set_highlight(
  const std::string& name,
  const ::nana::color& fgcolor,
  const ::nana::color& bgcolor)
{
  if (fgcolor.invisible() && fgcolor.invisible())  // <=
  {
    keywords_->schemes.erase(name);
    return;
  }
  ....
}

The analyzer detected two identical conditional expressions. This code was very likely written using Copy-Paste: the programmer copied the fgcolor.invisible() expression but forgot to change the fgcolor variable's name. Judging by the function's parameters, the programmer wanted to use argument bgcolor instead of fgcolor in the second subexpression. In that case, the fixed code should look like this:

void text_editor::set_highlight(
  const std::string& name,
  const ::nana::color& fgcolor,
  const ::nana::color& bgcolor)
{
  if (fgcolor.invisible() && bgcolor.invisible())
  {
    keywords_->schemes.erase(name);
    return;
  }
  ....
}

There's almost no way we can do without Copy-Paste when writing code, so you just have to be more careful when inspecting copy-pasted and amended code. If you have still made a mistake because of copy-paste, static analysis could help save time when tracking it down.

Using a null pointer (not a bug here, but code like that is always worth checking)

The analyzer detected a code fragment where a null pointer is used.

V522 Dereferencing of the null pointer 'debug' might take place. text_token_stream.hpp 669

This is what it looks like:

void parse(....)
{
  ....
  switch(tk)
  {
    ....
    default:
      int * debug = 0;  //for debug.
      *debug = 0;
  }
  ....
}

As the comment suggests, this trick with the debug pointer was done for debugging purposes, but I still felt I should cite this fragment as an example. You must be very careful with things like that, as program logic might change later and you'd be unpleasantly surprised. Anyway, this code should be re-checked just in case.

Incorrect use of a smart pointer

We have finally got to an issue related to the C++11 standard. The analyzer detected a situation where using a smart pointer might cause undefined behavior, in particular damaging the heap or a program crash. The reason is that the programmer uses different methods to allocate and free memory.

V554 Incorrect use of unique_ptr. The memory allocated with 'new []' will be cleaned using 'delete'. text_editor.cpp 3137

void text_editor::_m_draw_string(....) const
{
  ....
  for (auto & ent : reordered)
  {
    ....
    std::size_t len = ent.end - ent.begin;
    ....
    if (....)
    {
      ....
    }
    else if (pos <= a.x && a.x < str_end)
    {
      ....
      std::unique_ptr<unsigned> pxbuf_ptr(new unsigned[len]);  // <=
    }
  }
}

The unique_ptr class is used to manage the memory block allocated for the array. When freeing that block, it uses the delete operator by default, which results in undefined behavior. To fix this error, we need to use partial specialization of unique_ptr for the array. In that case, memory will be freed by calling the delete[] operator. This is what the fixed code should look like:

std::unique_ptr<unsigned[]> pxbuf_ptr(new unsigned[len]);

Redundant comparison

Sometimes conditional statements use redundant checks, which may contain potential errors. The analyzer detected a redundant comparison in the code and issued the following warning:

V590 Consider inspecting this expression. The expression is excessive or contains a misprint. window_manager.cpp 467

void window_manager::destroy_handle(core_window_t* wd)
{
  ....
  if((wd->other.category == category::root_tag::value) ||
     (wd->other.category != category::frame_tag::value))  // <=
  {
   impl_->misc_register.erase(wd->root);
   impl_->wd_register.remove(wd);
  }
}

Here's a simple example to explain the point:

if (a == 1 || a != 5)

The condition will execute if a != 5. The first part of the expression makes no sense. If you examine the code closely, you will come to one of the following conclusions: either the expression should be simplified by removing the first part - the code would then look like this:

if (a != 5)

or there is an error in the expression, in which case it should be fixed like this:

if (a == 1 || not_a_var != 5)

In our example, the first situation is more probable, so it should be simplified in the following way:

void window_manager::destroy_handle(core_window_t* wd)
{
  ....
  if(wd->other.category != category::frame_tag::value)
  {
   impl_->misc_register.erase(wd->root);
   impl_->wd_register.remove(wd);
  }
}

On dangerous uses of pointers one more time

You must be especially careful when using pointers in C/C++. If you are not sure there are any data found at the address pointed to by a pointer, make sure you verify it against null. Accessing memory pointed by a null pointer results in undefined behavior or a program crash.

V595 The 'owner' pointer was utilized before it was verified against nullptr. Check lines: 299, 315. window_manager.cpp 299

window_manager::core_window_t*
window_manager::create_root(core_window_t* owner,    )
{
  ....
  if (nested)
  {
    wd->owner = nullptr;
    wd->parent = owner;
    wd->index = static_cast<unsigned>(owner->children.size());
    owner->children.push_back(wd);  // <=
  }
  ....
  if (owner 
      && owner->other.category == category::frame_tag::value)  // <=
    insert_frame(owner, wd);
  ....
}

V595 is perhaps the most common warning across all the projects we check. Here's another similar issue found in this one:

V595 The 'wd' pointer was utilized before it was verified against nullptr. Check lines: 1066, 1083. window_manager.cpp 1066

Using the SuspendThread() function

The analyzer detected a code fragment where the SuspendThread() function is used:

V720 It is advised to utilize the 'SuspendThread' function only when developing a debugger (see documentation for details). pool.cpp 225

void _m_suspend(pool_throbj* pto)
{
  pto->thr_state = state::idle;
#if defined(NANA_WINDOWS)
  ::SuspendThread(pto->handle);  // <=
#elif defined(NANA_POSIX)
  std::unique_lock<std::mutex> lock(pto->wait_mutex);
  pto->suspended = true;
  pto->wait_cond.wait(lock);
  pto->suspended = false;
#endif
}

A call to this function is not an error in itself; however, developers often use it inappropriately, which might result in unexpected behavior. The SuspendThread() function is meant to help developers create debuggers and other similar utilities. If you use it for synching purposes, you are very likely to get a bug.

For more information on misuses of the SuspendThread() function, see the following articles:

Conclusion

Nana is a small project and there aren't many errors in it. However, some of the fragments do need to be checked. Among the errors found, there is one related to the use of the C++11 standard. One error is, of course, not enough to estimate PVS-Studio's capabilities for analyzing C++11 projects, so we would be glad to receive any suggestions from you. If you know any projects written in modern C++, please let us know, and we'll try to check them. Use the feedback form to contact us.

To sum up, I'd like to warn you against limiting yourself to tests and code review when writing program code. Using static analysis helps write better code and save time when searching for bugs. So, you are welcome to try PVS-Studio on your projects written in C, C++, or C#.

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