Cppcheck: More Checks

In my previous posts, I discussed some errors that Cppcheck can help you find. In this post I'm going to look at some more errors that Cppcheck can prevent.

Memory Leak from realloc

When using realloc, a failure to allocate data will return a null pointer. This can lead to a common error where the original data that you tried to reallocate is leaked. For example:

1
2
3
4
5
6
7
void realloc_issue() {
    int* data = (int*)malloc(4 * sizeof(int));
    // do something
    data = (int*)realloc(data, 8*sizeof(int));
    // do something
    free(data);
}

In the above code, the original data variable is leaked if the call to realloc fails. Using Cppcheck you will get the following warning about the code:

1
[realloc.cpp:4]: (error) Common realloc mistake: 'data' nulled but not freed upon failure

To fix this error you should allow a temporary to take the returned result from realloc and only assign it to data if it is valid memory:

1
2
3
int* new_data = (int*)realloc(data, 8*sizeof(int));
if(new_data)
    data = new_data;

Invalidated Iterator

When using stl containers such as std::vector, using methods such as push_back and resize can invalidate iterators. Cppcheck has a check which can help to catch this mistake. For example:

1
2
3
4
5
6
7
8
9
10
11
void pb_it() {
    std::vector<std::string> v;
    v.push_back("first");
 
    std::vector<std::string>::iterator first = v.begin();
    for(int i = 0; i < 100; ++i) {
        v.push_back("loop");
    }
 
    std::cout << *first << "\n";
}

This will result in the following warning:

1
[push_back.cpp:10]: (error) After push_back(), the iterator 'first' may be invalid.

Note: If you use auto for the iterator or a standard algorithm to fill your vector, then Cppcheck will fail to warn about the error. For example, neither of the following will produce a warning:

1
2
3
std::vector<std::string>::iterator first = v.begin();
std::fill_n(std::back_insertor(v), 100, "loop");
std::cout << *first << "\n";

or

auto first = v.begin(); for ... std::cout << *first << "\n";

Invalid Operation on File

Using the C library functions for working on a FILE*, there is no type safe way at compile time to catch code that reads from a write only file, or writes to a read only file. However, Cppcheck has warnings which can catch this error.

1
2
3
4
5
6
7
8
9
10
11
12
13
void read_on_write_only() {
    FILE* f = fopen("xxxw", "w");
    std::array<char, 100> buf;
    fread(buf.data(), 1, 100, f);
    fclose(f);
}   
 
void write_on_read_only() {
    FILE* f = fopen("xxxf", "r");
    int x = 0;
    fwrite(&x, sizeof(int), 1, f);
    fclose(f);
}   

In the above code we use fread on a file that was opened as write only, and fwrite on a file that was opened read only. Cppcheck will output the following warnings:

1
2
[file_ops.cpp:4]: (error) Read operation on a file that was opened only for writing.
[file_ops.cpp:11]: (error) Write operation on a file that was opened only for reading.

Note: This doesn't warn with std::fstream. For example, If you open a file with ios_base::in, and try to write to it then Cppcheck will fail to warn.

Class Checks

Cppcheck has a number of both style and warning checks that can hint at potential problems with your class design. For example, take this poorly designed class:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
#include <string>
 
class Multipler
{
public:
    Multipler(int x) : x_(new int(x))
    {
    }
    ~Multipler() = default;
    Multipler& operator=(const Multipler& rhs) {
        x_ = new int(*rhs.x_);
        return *this;
    }
    int multiply(int y) const { return y * (*x_); }
 
    void name(const std::string& name) { name_ = name; }
private:
    int* x_;
    std::string name_; 
    int vx_;
};

On most compilers this will compile with no warnings. However, Cppcheck will output the following warnings:

1
2
3
4
5
6
7
[class_design.cpp:6]: (warning) Member variable 'Multipler::vx_' is not initialized in the constructor.
[class_design.cpp:10]: (warning) Member variable 'Multipler::vx_' is not assigned a value in 'Multipler::operator='.
[class_design.cpp:10]: (warning) 'operator=' should check for assignment to self to avoid problems with dynamic memory.
[class_design.cpp:6]: (style) Class 'Multipler' does not have a copy constructor which is recommended since it has dynamic memory/resource allocation(s).
[class_design.cpp:6]: (style) Class 'Multipler' has dynamic memory/resource allocation(s). The destructor is explicitly defaulted but the default destructor does not work well. It is recommended to define the destructor.
[class_design.cpp:6]: (style) Class 'Multipler' has a constructor with 1 argument that is not explicit.
[class_design.cpp:11]: (warning) Possible leak in public function. The pointer 'x_' is not deallocated before it is allocated.

These are good checks, however, you need to have style warnings enabled for some of them. If you only use enable=warning, Cppcheck will not warn about the fact that you fail to delete x_ in the destructor.

Conclusions about Cppcheck

Cppcheck is a good tool to help you write more robust and safe code. However, it is not a silver bullet to prevent errors. As noted in some of my examples, a small change to your code can mean that Cppcheck will fail to catch an error.

I have found that Cppcheck can be very helpful when taking over an old codebase that uses an older style of C++. After enabling Cppcheck, you can often catch errors that have existed in the code for years and the developers have been lucky not to hit (e.g. mistakes in error handlers). If you are using more modern C++ style, Cppcheck might not be up to date with all changes and may miss some warnings. However, I would still recommend to have it running in your CI system to prevent errors from reaching your production code.