USCiLab / cereal

A C++11 library for serialization
BSD 3-Clause "New" or "Revised" License
4.17k stars 751 forks source link

Static analyzer findings #594

Open rsjaffe opened 4 years ago

rsjaffe commented 4 years ago

I turned on a static analyzer I use (PVS-Studio) and checked the portions of cereal that are used by my applications. There were a couple of alerts on rapidxml and xml—since I believe those are external to your project I'm not listing them here.

The following alerts seem to be significant:

V714 Variable 'v' is not passed into foreach loop by a reference, but its value is changed inside of the loop.

vector.hpp lines 103-108. The assignment v = b looks like it doesn't do anything:

    for(auto v : vector)
    {
      bool b;
      ar( b );
      v = b;
    }

V690 Copy constructor is declared as private in the 'StaticObject' class, but the default '=' operator will still be generated by compiler. It is dangerous to use such a class.

This was triggered by line 67 of static_object.hpp (class CEREAL_DLL_EXPORT StaticObject).

AzothAmmo commented 4 years ago

Those lines in vector.hpp are operating on a vector<bool> which is a bit of a special case, since the iterator returns a temporary object which can be used to manipulate the vector. We could however use a "universal reference" (i.e., &&) which would not collapse to an l-value reference and probably make the analyzer happy. It won't change the behavior here, however.

For the other one, see if making the assignment operator private satisfies the analysis.