Which C++ Bugs Appear Again and Again in Code Review?

The C++ bugs that appear again and again in code review are mostly not clever mistakes. They are the same handful of problems: a reference that outlives its target, an iterator that was invalidated, an index that is one too large, a signed value that overflowed, and a copy that was forgotten. Learning to spot them is a matter of knowing the list and checking against it.

The dangling reference

The most common serious bug in C++ is a reference to something that no longer exists. It usually arrives through a function that returns a reference to a local.

#include 
#include 

const std::string& bad_label() {
    std::string label = "perishable";
    return label;           // the local dies when the function returns
}

int main() {
    const std::string& label = bad_label();
    std::cout << "label: " << label << '\n';
    return 0;
}

The output may look correct, which is the problem: the stack slot is usually still there and still holds the bytes. It may also be garbage, or it may crash. The fix is to return by value.

#include 
#include 

std::string good_label() {
    std::string label = "perishable";
    return label;           // the value is copied out, nothing dangles
}

int main() {
    std::string label = good_label();
    std::cout << "label: " << label << '\n';
    return 0;
}

The output is now defined:

label: perishable

The same bug appears with a reference to a vector element that a later push_back reallocates, and with a reference to a member of a temporary.

Iterator invalidation

Adding to or removing from a container invalidates the iterators into it. The code compiles and then misbehaves.

#include 
#include 
#include 

int main() {
    std::vector items = {"a", "b"};
    std::vector::iterator it = items.begin();

    items.push_back("c");      // may reallocate

    std::cout << "first: " << *it << '\n';
    return 0;
}

The fix is either to reserve the space first, to use an index instead of an iterator, or to recompute the iterator after the change.

#include 
#include 
#include 

int main() {
    std::vector items = {"a", "b"};
    items.reserve(10);         // no reallocation will happen now

    std::vector::iterator it = items.begin();
    items.push_back("c");

    std::cout << "first: " << *it << '\n';
    return 0;
}

The output is now reliable:

first: a

Signed integer overflow

Signed overflow is undefined behaviour, and it usually shows up as a negative quantity where a positive one was expected.

#include 

int main() {
    int total = 2000000000;
    int doubled = total * 2;     // overflow: undefined behaviour

    std::cout << "doubled: " << doubled << '\n';
    return 0;
}

The fix is to use a type that can hold the result.

#include 

int main() {
    int total = 2000000000;
    long long doubled = static_cast(total) * 2;

    std::cout << "doubled: " << doubled << '\n';
    return 0;
}

The output shows the correct value:

doubled: 4000000000

The rule is to promote before you multiply, not after.

Off-by-one errors

The loop that runs one time too often, or one time too few, is the most common logic bug there is. The classic is using <= where < was meant.

#include 

int main() {
    int stock[5] = {10, 20, 30, 40, 50};
    int count = 5;
    int total = 0;

    for (int i = 0; i <= count; ++i) {      // reads stock[5], past the end
        total += stock[i];
    }

    std::cout << "total: " << total << '\n';
    return 0;
}

The loop condition should be i < count. A habit that prevents the bug is to write the range as a half-open interval in a comment, or to use a range-based for whenever the index is not needed.

#include 

int main() {
    int stock[5] = {10, 20, 30, 40, 50};
    int total = 0;

    for (int value : stock) {
        total += value;
    }

    std::cout << "total: " << total << '\n';
    return 0;
}

The output is correct:

total: 150

The accidental copy

Passing a large object by value when a reference was intended copies it on every call, and for a type that owns a resource it can also change the ownership semantics.

#include 
#include 

class Label {
public:
    explicit Label(std::string text) : text_(std::move(text)) {
        std::cout << "copy made\n";
    }

    const std::string& text() const { return text_; }

private:
    std::string text_;
};

void print_label(Label label) {          // by value: a copy every call
    std::cout << label.text() << '\n';
}

void print_label_ref(const Label& label) {   // by const reference: no copy
    std::cout << label.text() << '\n';
}

int main() {
    Label label("perishable");
    print_label(label);
    print_label_ref(label);
    return 0;
}

The output shows the extra copy:

copy made
perishable
perishable

Passing by const reference removes the copy and the allocation that goes with it.

Ownership confusion in a class

A class that holds a raw pointer without saying whether it owns the pointee is a design smell that leads to leaks and double frees.

#include 
#include 

class Session {
public:
    explicit Session(std::unique_ptr id) : id_(std::move(id)) {}
    ~Session() = default;

    Session(const Session&) = delete;
    Session& operator=(const Session&) = delete;

    int id() const { return *id_; }

private:
    std::unique_ptr id_;
};

int main() {
    auto id = std::make_unique(4821);
    Session session(std::move(id));

    std::cout << "session id: " << session.id() << '\n';
    return 0;
}

The output shows the session owning its identifier:

session id: 4821

By storing a std::unique_ptr and deleting the copy operations, the class says exactly what it means: one owner, no copies, freed automatically.

A review checklist

• Does any reference or pointer outlive the object it points at?
• Does any iterator survive a modification to its container?
• Can any arithmetic overflow the type it is stored in?
• Are the loop bounds right, and does the empty case work?
• Are large objects passed by const reference rather than by value?
• Does every owning class have an unambiguous owner, and a destructor that matches?
• Are the warnings enabled, and is the code clean under them?

Quick reference

• Dangling references to locals are the most common serious bug.
• Reallocation invalidates every iterator and reference into a container.
• Signed overflow is undefined behaviour; promote before multiplying.
• Prefer < over <= and use range-based for where possible.
• Pass large objects by const reference.
• A class that owns a resource should own it with a smart pointer.
• Turn on -Wall -Wextra and read every warning.

Try it yourself: take a piece of code you have written and check it against the checklist, one item at a time. Compile it with the sanitizers and with [[-Wall -Wextra]], and fix whatever they report. Then write a function that returns a reference to a local and confirm with the sanitizer that the use-after-free is detected on the exact line.