Checking a collection of header-only C ++ libraries (awesome-hpp)

PVS-Studio and Awesome hpp


By a twist of fate, we have checked most of the libraries included in the "Awesome hpp" collection. These are small C ++ projects that only consist of header files. Hopefully the bugs found will help make these libraries a little better. We will also be glad if their authors start using the PVS-Studio analyzer for free on a regular basis.



I bring to your attention an overview of the results of testing various libraries listed in the awesome-hpp (A curated list of awesome header-only C ++ libraries) list.



I first learned about this list from the " Cross Platform Mobile Telephony " podcast . Taking this opportunity, I recommend all C ++ programmers to get acquainted with CppCast . CppCast is the first podcast for C ++ developers by C ++ developers!



Despite the large number of projects on the list, there were very few errors. There are three reasons for this:



  • These are very small projects. Many literally consist of a single header file.
  • We have not checked all projects. There were problems with compilation of some of them and we decided to skip them.
  • Often, in order to understand if there are errors in template classes / functions or not, they must be instantiated. Accordingly, many errors can be detected by the analyzer only in this project, when the library is actively used. We just included the header files in an empty .cpp file and checked, which makes checking ineffective.


Nevertheless, in the process of studying the warnings, there were enough of them to write this article and a couple of additional ones.



Note to my colleagues
, - . . awesome-hpp, :



  • , C++11, C++14 C++17;
  • " , ";
  • " — , ";
  • ;
  • (. CSV Parser);
  • , . — , - :);
  • , .




A note for library developers. Those interested can use the PVS-Studio analyzer for free to check open source projects. To obtain a license for your open source project, please fill out this form .



Now let's finally take a look at what was found in some of the libraries.



Found errors



Iutest library



Brief description of the iutest library :
iutest is framework for writing C ++ tests.
template<typename Event>
pool_handler<Event> & assure() {
  ....
  return static_cast<pool_handler<Event> &>(it == pools.cend() ?
    *pools.emplace_back(new pool_handler<Event>{}) : **it);
  ....
}


PVS-Studio warning : V1023 A pointer without owner is added to the 'pools' container by the 'emplace_back' method. A memory leak will occur in case of an exception. entt.hpp 17114



This code has the potential to leak memory. If the container needs reallocation and it cannot allocate memory for a new array, it will throw an exception and the pointer will be lost.



Perhaps, for tests, this situation is unlikely and not critical. However, I decided to mention this shortcoming for educational purposes :).



Correct option:



pools.emplace_back(std::make_unique<pool_handler<Event>>{})


Another similar place: V1023 A pointer without owner is added to the 'pools' container by the 'emplace_back' method. A memory leak will occur in case of an exception. entt.hpp 17407



Jsoncons library



A short description of the jsoncons library :
A C ++, header-only library for constructing JSON and JSON-like data formats, with JSON Pointer, JSON Patch, JSONPath, JMESPath, CSV, MessagePack, CBOR, BSON, UBJSON.
The first mistake



static constexpr uint64_t basic_type_bits = sizeof(uint64_t) * 8;

uint64_t* data() 
{
  return is_dynamic() ? dynamic_stor_.data_ : short_stor_.values_;
}

basic_bigint& operator<<=( uint64_t k )
{
  size_type q = (size_type)(k / basic_type_bits);
  ....
  if ( k )  // 0 < k < basic_type_bits:
  {
    uint64_t k1 = basic_type_bits - k;
    uint64_t mask = (1 << k) - 1;             // <=
    ....
    data()[i] |= (data()[i-1] >> k1) & mask;
    ....
  }
  reduce();
  return *this;
}


PVS-Studio warning : V629 Consider inspecting the '1 << k' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. bigint.hpp 744



This error has already been discussed in detail in the article " Why it is important to carry out static analysis of open source libraries that you add to your project ". In short, in order to get the correct mask values, you need to write like this:



uint64_t mask = (static_cast<uint64_t>(1) << k) - 1;


Or like this:



uint64_t mask = (1ull << k) - 1;


The exact same error as the first can be seen here: V629 Consider inspecting the '1 << k' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. bigint.hpp 779



Second error



template <class CharT = typename std::iterator_traits<Iterator>::value_type>
typename std::enable_if<sizeof(CharT) == sizeof(uint16_t)>::type 
next() UNICONS_NOEXCEPT
{
    begin_ += length_;
    if (begin_ != last_)
    {
        if (begin_ != last_)
        {
  ....
}


PVS-Studio warning : V571 Recurring check. The 'if (begin_! = Last_)' condition was already verified in line 1138. unicode_traits.hpp 1140



Strange retest . There is a suspicion that there is some typo here and the second condition should look something different.



Clipp library



A short description of the clipp library :
clipp - command line interfaces for modern C ++. Easy to use, powerful and expressive command line argument handling for C ++ 11/14/17 contained in a single header file.
inline bool
fwd_to_unsigned_int(const char*& s)
{
  if(!s) return false;
  for(; std::isspace(*s); ++s);
  if(!s[0] || s[0] == '-') return false;
  if(s[0] == '-') return false;
  return true;
}


PVS-Studio warning : V547 Expression 's [0] ==' - '' is always false. clipp.h 303



Well, in fact, this is not an error, but simply redundant code. The minus check is performed twice.



SimpleIni library



A short description of the SimpleIni library :
A cross-platform library that provides a simple API to read and write INI-style configuration files. It supports data files in ASCII, MBCS and Unicode.
#if defined(SI_NO_MBSTOWCS_NULL) || (!defined(_MSC_VER) && !defined(_linux))


PVS-Studio warning : V1040 Possible typo in the spelling of a pre-defined macro name. The '_linux' macro is similar to '__linux'. SimpleIni.h 2923



Most likely, there is one underscore missing in the _linux macro name and the name __linux should be used . However, in POSIX this macro is deprecated and it is better to use __linux__ .



CSV Parser Library



A short description of the CSV Parser library :
A modern C ++ library for reading, writing, and analyzing CSV (and similar) files.
CSV_INLINE void CSVReader::read_csv(const size_t& bytes) {
  const size_t BUFFER_UPPER_LIMIT = std::min(bytes, (size_t)1000000);
  std::unique_ptr<char[]> buffer(new char[BUFFER_UPPER_LIMIT]);
  auto * HEDLEY_RESTRICT line_buffer = buffer.get();
  line_buffer[0] = '\0';
  ....
  this->feed_state->feed_buffer.push_back(
    std::make_pair<>(std::move(buffer), line_buffer - buffer.get())); // <=
  ....
}


PVS-Studio warning : V769 The 'buffer.get ()' pointer in the 'line_buffer - buffer.get ()' expression equals nullptr. The resulting value is senseless and it should not be used. csv.hpp 4957



An interesting situation that requires careful consideration. Therefore, I decided that I would write a separate small note about this. Plus, while experimenting with similar code, I found a flaw in PVS-Studio itself :). In some cases, it is silent, although it should issue warnings.



In short, whether this code works or not depends on the order in which the arguments are evaluated. The order in which the arguments are evaluated depends on the compiler.



PPrint Library



Brief description of the library PPRINT :.
Pretty Printer for Modern C ++.
template <typename Container>
typename std::enable_if<......>::type print_internal(......) {
  ....
  for (size_t i = 1; i < value.size() - 1; i++) {
    print_internal(value[i], indent + indent_, "", level + 1);
    if (is_container<T>::value == false)
      print_internal_without_quotes(", ", 0, "\n");
    else
      print_internal_without_quotes(", ", 0, "\n");
  }
  ....
}


PVS-Studio warning : V523 The 'then' statement is equivalent to the 'else' statement. pprint.hpp 715



It is very strange that the same action is performed regardless of the condition. There is no special explanatory commentary either. This is all very similar to the Copy-Paste error.



Similar warnings:



  • V523 The 'then' statement is equivalent to the 'else' statement. pprint.hpp 780
  • V523 The 'then' statement is equivalent to the 'else' statement. pprint.hpp 851
  • V523 The 'then' statement is equivalent to the 'else' statement. pprint.hpp 927
  • V523 The 'then' statement is equivalent to the 'else' statement. pprint.hpp 1012


Strf library



A short description of the Strf library :
A fast C ++ formatting library that supports encoding conversion.
The first mistake



template <int Base>
class numpunct: private strf::digits_grouping
{
  ....
  constexpr STRF_HD numpunct& operator=(const numpunct& other) noexcept
  {
    strf::digits_grouping::operator=(other);
    decimal_point_ = other.decimal_point_;
    thousands_sep_ = other.thousands_sep_;
  }
  ....
};


PVS-Studio warning : V591 Non-void function should return a value. numpunct.hpp 402



We forgot to write "return * this;" at the end of the function.



The second similar error



template <int Base>
class no_grouping final
{
  constexpr STRF_HD no_grouping& operator=(const no_grouping& other) noexcept
  {
    decimal_point_ = other.decimal_point_;
  }
  ....
}


PVS-Studio warning : V591 Non-void function should return a value. numpunct.hpp 528.



Indicators library



Brief description of the Indicators library :
Activity Indicators for Modern C ++.
static inline void move_up(int lines) { move(0, -lines); }
static inline void move_down(int lines) { move(0, -lines); }   // <=
static inline void move_right(int cols) { move(cols, 0); }
static inline void move_left(int cols) { move(-cols, 0); }


PVS-Studio warning : V524 It is odd that the body of 'move_down' function is fully equivalent to the body of 'move_up' function. indicators.hpp 983



I'm not sure if this is a bug. But the code is very suspicious. It is highly likely that the move_up function was copied and its name changed to move_down . But they forgot to remove the minus. It's worth checking out this code.



Note. If the code is correct, you need to understand that it misleads not only code analyzers, but also third-party programmers who want to use or develop this code. It is useful to add comments to this code.



Manif library



A short description of the manif library :
manif is a header-only C ++ 11 Lie theory library for state-estimation targeted at robotics applications.
template <typename _Derived>
typename LieGroupBase<_Derived>::Scalar*
LieGroupBase<_Derived>::data()
{
  return derived().coeffs().data();
}

template <typename _Derived>
const typename LieGroupBase<_Derived>::Scalar*
LieGroupBase<_Derived>::data() const
{
  derived().coeffs().data(); // <=
}


PVS-Studio warning : V591 Non-void function should return a value. lie_group_base.h 347 The



non-constant function is implemented correctly, but the constant function is not. It's even interesting how it happened ...



FakeIt Library



A short description of the FakeIt library :
FakeIt is a simple mocking framework for C ++. It supports GCC, Clang and MS Visual C ++. FakeIt is written in C ++ 11 and can be used for testing both C ++ 11 and C ++ projects.
template<typename ... arglist>
struct ArgumentsMatcherInvocationMatcher :
         public ActualInvocation<arglist...>::Matcher {
  ....
  template<typename A>
  void operator()(int index, A &actualArg) {
      TypedMatcher<typename naked_type<A>::type> *matcher =
        dynamic_cast<TypedMatcher<typename naked_type<A>::type> *>(
          _matchers[index]);
      if (_matching)
        _matching = matcher->matches(actualArg);
  }
  ....
  const std::vector<Destructible *> _matchers;
};


PVS-Studio warning : V522 There might be dereferencing of a potential null pointer 'matcher'. fakeit.hpp 6720



The matcher pointer is initialized with the value returned by the dynamic_cast operator . And this operator can return nullptr, and this is a very likely scenario. Otherwise, it is more efficient to use static_cast instead of dynamic_cast . There is a suspicion that there is a typo in the condition and in fact it should be written:







if (matcher)
  _matching = matcher->matches(actualArg);


GuiLite Library



A short description of the GuiLite library :
The smallest header-only GUI library (4 KLOC) for all platforms.
#define CORRECT(x, high_limit, low_limit)  {\
  x = (x > high_limit) ? high_limit : x;\
  x = (x < low_limit) ? low_limit : x;\
}while(0)

void refresh_wave(unsigned char frame)
{
  ....
  CORRECT(y_min, m_wave_bottom, m_wave_top);
  ....
}


PVS-Studio warning : V529 Odd semicolon ';' after 'while' operator. GuiLite.h 3413



An error in a macro does not lead to some problem. But it's still a mistake, so I decided to describe it in the article.



It was planned to use the classic pattern do {...} while (....) in the macro . This allows you to perform several actions in one block and at the same time be able to write a semicolon ';' after the macro for beauty, as if it were a function call.



But in the considered macro, they accidentally forgot to write the do keyword . As a result, the macro is, as it were, divided into two parts. The first is the block. The second is an empty, non-executing loop: while (0); ...



And what is the problem?



For example, such a macro cannot be used in a construction like:



if (A)
  CORRECT(y_min, m_wave_bottom, m_wave_top);
else
  Foo();


This code will not compile as it will be expanded into:



if (A)
  { ..... }
while(0);
else
  Foo();


Agree, it is better to find and fix such a problem at the stage of library development, and not at the stage of using it. Apply static code analysis :).





PpluX library



Brief description of the PpluX library :
Single header C ++ Libraries for Thread Scheduling, Rendering, and so on ...
struct DisplayList {
  DisplayList& operator=(DisplayList &&d) {
    data_ = d.data_;
    d.data_ = nullptr;
  }
  ....
}


PVS-Studio warning : V591 Non-void function should return a value. px_render.h 398



Universal Library



A short description of the Universal library:
The goal of Universal Numbers, or unums, is to replace IEEE floating-point with a number system that is more efficient and mathematically consistent in concurrent execution environments.
The first mistake



template<typename Scalar>
vector<Scalar> operator*(double scalar, const vector<Scalar>& v) {
  vector<Scalar> scaledVector(v);
  scaledVector *= scalar;
  return v;
}


PVS-Studio warning : V1001 The 'scaledVector' variable is assigned but is not used by the end of the function. vector.hpp 124 Typo



. Instead of the original vector v, the function needs to return a new vector, scaledVector .



A similar typo can be seen here: V1001 The 'normalizedVector' variable is assigned but is not used by the end of the function. vector.hpp 131



Second error



template<typename Scalar>
class matrix {
  ....
  matrix& diagonal() {
  }
  ....
};


PVS-Studio warning : V591 Non-void function should return a value. matrix.hpp 109



Third error



template<size_t fbits, size_t abits>
void module_subtract_BROKEN(
  const value<fbits>& lhs, const value<fbits>& rhs, value<abits + 1>& result)
{
  if (lhs.isinf() || rhs.isinf()) {
    result.setinf();
    return;
  }
  int lhs_scale = lhs.scale(),
      rhs_scale = rhs.scale(),
      scale_of_result = std::max(lhs_scale, rhs_scale);

  // align the fractions
  bitblock<abits> r1 =
    lhs.template nshift<abits>(lhs_scale - scale_of_result + 3);
  bitblock<abits> r2 =
    rhs.template nshift<abits>(rhs_scale - scale_of_result + 3);
  bool r1_sign = lhs.sign(), r2_sign = rhs.sign();
  //bool signs_are_equal = r1_sign == r2_sign;

  if (r1_sign) r1 = twos_complement(r1);
  if (r1_sign) r2 = twos_complement(r2);  // <=

  ....
}


PVS-Studio warning : V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 789, 790. value.hpp 790



A classic error caused by Copy-Paste. They took and multiplied the line:



if (r1_sign) r1 = twos_complement(r1);


We changed r1 to r2 in it :



if (r1_sign) r2 = twos_complement(r2);


And they forgot to change r1_sign . Correct option:



if (r2_sign) r2 = twos_complement(r2);


Chobo Single-Header Libraries



A short description of the Chobo Single-Header Libraries :
A collection of small single-header C ++ 11 libraries by Chobolabs.
The first mistake



template <typename T, typename U, typename Alloc = std::allocator<T>>
class vector_view
{
  ....
  vector_view& operator=(vector_view&& other)
  {
    m_vector = std::move(other.m_vector);
  }
  ....
}


PVS-Studio warning : V591 Non-void function should return a value. vector_view.hpp 163



Second error



template <typename UAlloc>
vector_view& operator=(const std::vector<U, UAlloc>& other)
{
  size_type n = other.size();
  resize(n);
  for (size_type i = 0; i < n; ++i)
  {
    this->at(i) = other[i];
  }
}


PVS-Studio warning : V591 Non-void function should return a value. vector_view.hpp 184



Library PGM-index



Brief description of the PGM-index library :
The Piecewise Geometric Model index (PGM-index) is a data structure that enables fast lookup, predecessor, range searches and updates in arrays of billions of items using orders of magnitude less space than traditional indexes while providing the same worst-case query time guarantees ...
The first mistake



char* str_from_errno()
{
#ifdef MSVC_COMPILER
  #pragma warning(disable:4996)
  return strerror(errno);
#pragma warning(default:4996)
#else
  return strerror(errno);
#endif
}


PVS-Studio warning : V665 Possibly, the usage of '#pragma warning (default: X)' is incorrect in this context. The '#pragma warning (push / pop)' should be used instead. Check lines: 9170, 9172. sdsl.hpp 9172



Incorrect temporarily disabling compiler warning. Such inaccuracies are somehow forgivable to user code. But this is definitely not allowed in header-only libraries.



Second mistake



template<class t_int_vec>
t_int_vec rnd_positions(uint8_t log_s, uint64_t& mask,
                        uint64_t mod=0, uint64_t seed=17)
{
  mask = (1<<log_s)-1;         // <=
  t_int_vec rands(1<<log_s ,0);
  set_random_bits(rands, seed);
  if (mod > 0) {
    util::mod(rands, mod);
  }
  return rands;
}


PVS-Studio warning : V629 Consider inspecting the '1 << log_s' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. sdsl.hpp 1350



One of the correct options:



mask = ((uint64_t)(1)<<log_s)-1;


Hnswlib library



A short description of the Hnswlib library :
Header-only C ++ HNSW implementation with python bindings. Paper's code for the HNSW 200M SIFT experiment.
template<typename dist_t>
class BruteforceSearch : public AlgorithmInterface<dist_t> {
public:
  BruteforceSearch(SpaceInterface <dist_t> *s, size_t maxElements) {
    maxelements_ = maxElements;
    data_size_ = s->get_data_size();
    fstdistfunc_ = s->get_dist_func();
    dist_func_param_ = s->get_dist_func_param();
    size_per_element_ = data_size_ + sizeof(labeltype);
    data_ = (char *) malloc(maxElements * size_per_element_);
    if (data_ == nullptr)
      std::runtime_error(
        "Not enough memory: BruteforceSearch failed to allocate data");
    cur_element_count = 0;
  }
  ....
}


PVS-Studio warning : V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw runtime_error (FOO); bruteforce.h 26



Forgot to write a throw statement before std :: runtime_error . Another such error: V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw runtime_error (FOO); bruteforce.h 161







The tiny-dnn library



A short description of the tiny-dnn library :
tiny-dnn is a C ++ 14 implementation of deep learning. It is suitable for deep learning on limited computational resource, embedded systems and IoT devices.
The first mistake



class nn_error : public std::exception {
 public:
  explicit nn_error(const std::string &msg) : msg_(msg) {}
  const char *what() const throw() override { return msg_.c_str(); }

 private:
  std::string msg_;
};

inline Device::Device(device_t type, const int platform_id, const int device_id)
  : type_(type),
    has_clcuda_api_(true),
    platform_id_(platform_id),
    device_id_(device_id) {
  ....
#else
  nn_error("TinyDNN has not been compiled with OpenCL or CUDA support.");
#endif
}


PVS-Studio warning : V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw nn_error (FOO); device.h 68



nn_error is not a function that throws an exception, but just a class. Therefore, it is correct to use it like this:



throw nn_error("TinyDNN has not been compiled with OpenCL or CUDA support.");


Another misuse of this class: V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw nn_error (FOO); conv2d_op_opencl.h 136



Second error



inline std::string format_str(const char *fmt, ...) {
  static char buf[2048];

#ifdef _MSC_VER
#pragma warning(disable : 4996)
#endif
  va_list args;
  va_start(args, fmt);
  vsnprintf(buf, sizeof(buf), fmt, args);
  va_end(args);
#ifdef _MSC_VER
#pragma warning(default : 4996)
#endif
  return std::string(buf);
}


PVS-Studio warning : V665 Possibly, the usage of '#pragma warning (default: X)' is incorrect in this context. The '#pragma warning (push / pop)' should be used instead. Check lines: 139, 146. util.h 146



Dlib library



A short description of the Dlib library :
Dlib is a modern C ++ toolkit containing machine learning algorithms and tools for creating complex software in C ++ to solve real world problems.
First error



For fun, try to find this error yourself.



class bdf_parser
{
public:

  enum bdf_enums
  {
    NO_KEYWORD = 0,
    STARTFONT = 1,
    FONTBOUNDINGBOX = 2,
    DWIDTH = 4,
    DEFAULT_CHAR = 8,
    CHARS = 16,
    STARTCHAR = 32,
    ENCODING = 64,
    BBX = 128,
    BITMAP = 256,
    ENDCHAR = 512,
    ENDFONT = 1024
  };
  ....
  bool parse_header( header_info& info )
  {
    ....
    while ( 1 )
    {
      res = find_keywords( find | stop );
      if ( res & FONTBOUNDINGBOX )
      {
          in_ >> info.FBBx >> info.FBBy >> info.Xoff >> info.Yoff;
          if ( in_.fail() )
              return false;    // parse_error
          find &= ~FONTBOUNDINGBOX;
          continue;
      }
      if ( res & DWIDTH )
      {
          in_ >> info.dwx0 >> info.dwy0;
          if ( in_.fail() )
              return false;    // parse_error
          find &= ~DWIDTH;
          info.has_global_dw = true;
          continue;
      }
      if ( res & DEFAULT_CHAR )
      {
          in_ >> info.default_char;
          if ( in_.fail() )
              return false;    // parse_error
          find &= ~DEFAULT_CHAR;
          continue;
      }
      if ( res & NO_KEYWORD )
          return false;    // parse_error: unexpected EOF
      break;
    }
  ....
};


Found it?



Confused Travolta-Unicorn


She's here:



if ( res & NO_KEYWORD )


PVS-Studio warning : V616 The 'NO_KEYWORD' named constant with the value of 0 is used in the bitwise operation. fonts.cpp 288 The



named constant NO_KEYWORD has the value 0. Therefore, the condition is meaningless. It would be correct to write:



if ( res == NO_KEYWORD )


Another incorrect check is found here: V616 The 'NO_KEYWORD' named constant with the value of 0 is used in the bitwise operation. fonts.cpp 334



Second error



void set(std::vector<tensor*> items)
{
  ....
  epa.emplace_back(new enable_peer_access(*g[0], *g[i]));
  ....
}


PVS-Studio warning : V1023 A pointer without owner is added to the 'epa' container by the 'emplace_back' method. A memory leak will occur in case of an exception. tensor_tools.h 1665



To understand where the catch is, I propose to get acquainted with the documentation for the V1023 diagnostics .



Third mistake



template <
    typename detection_type, 
    typename label_type 
    >
bool is_track_association_problem (
  const std::vector<
    std::vector<labeled_detection<detection_type,label_type> > >& samples
)
{
  if (samples.size() == 0)
    return false;

  unsigned long num_nonzero_elements = 0;
  for (unsigned long i = 0; i < samples.size(); ++i)
  {
    if (samples.size() > 0)
      ++num_nonzero_elements;
  }
  if (num_nonzero_elements < 2)
    return false;
  ....
}


PVS-Studio warning : V547 Expression 'samples.size ()> 0' is always true. svm.h 360



This is very, very strange code! If a loop starts, then the condition (samples.size ()> 0) is always true. Hence, the loop can be simplified:



for (unsigned long i = 0; i < samples.size(); ++i)
{
  ++num_nonzero_elements;
}


After that, it becomes clear that the loop is not needed at all. It can be written much easier and more efficiently:



unsigned long num_nonzero_elements = samples.size();


But was it planned to be done? The code clearly deserves careful study by a programmer.



The fourth mistake



class console_progress_indicator
{
  ....
  double seen_first_val;
  ....
};

bool console_progress_indicator::print_status (
  double cur, bool always_print)
{
  ....
  if (!seen_first_val)
  {
    start_time = cur_time;
    last_time = cur_time;
    first_val = cur;
    seen_first_val = true;  // <=
    return false;
  }
  ....
}


PVS-Studio warning : V601 The bool type is implicitly cast to the double type. console_progress_indicator.h 136



The value true is written to a member of the class of type double . Hmm ... Fifth mistake







void file::init(const std::string& name)
{
  ....
  WIN32_FIND_DATAA data;
  HANDLE ffind = FindFirstFileA(state.full_name.c_str(), &data);
  if (ffind == INVALID_HANDLE_VALUE ||
      (data.dwFileAttributes&FILE_ATTRIBUTE_DIRECTORY) != 0)
  {
    throw file_not_found("Unable to find file " + name);                
  }
  else
  {
    ....
  } 
}


PVS-Studio warning : V773 The exception was thrown without closing the file referenced by the 'ffind' handle. A resource leak is possible. dir_nav_kernel_1.cpp 60



If a directory is found, an exception is thrown. But who will close the handle?



The sixth mistake



Another very strange place.



inline double poly_min_extrap(double f0, double d0,
                              double x1, double f_x1,
                              double x2, double f_x2)
{
  ....
  matrix<double,2,2> m;
  matrix<double,2,1> v;

  const double aa2 = x2*x2;
  const double aa1 = x1*x1;
  m =  aa2,       -aa1,
      -aa2*x2, aa1*x1;   
  v = f_x1 - f0 - d0*x1,
      f_x2 - f0 - d0*x2;
  ....
}


PVS-Studio warning : V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. optimization_line_search.h 211



Scheduled to initialize matrices. But all these aa2 , f_x1 , d0 and so on are just variables of the double type . This means that commas do not separate the arguments intended to create matrices, but are ordinary comma operators that return the value of the right-hand side.



Conclusion



At the beginning of the article, I gave an example of how you can combine several useful things at once. Using a static analyzer is also useful at the same time for several reasons:



  • Training. By studying the analyzer warnings, you can learn a lot of new and useful things. Examples: memset , #pragma warning , emplace_back , strictly aligned .
  • Early detection of typos, bugs and potential vulnerabilities.
  • The code is gradually becoming better, simpler, more understandable.
  • You can be proud and tell everyone that you are using modern technologies when developing projects :). And this is only partly humor. This is a real competitive advantage.


The only question is how to start, how to implement it painlessly and how to use it correctly? The following articles will help you with this:







If you want to share this article with an English-speaking audience, please use the translation link: Andrey Karpov. Checking a Header-Only C ++ Library Collection (awesome-hpp) .



All Articles