Common mistakes (C++)

The following list shows a few common mistakes you will find in code you will have to review before its check-in.

  1. Calling a method of an object behind a raw pointer without checking on nullptr before
    • Details: If you aren’t setting the raw pointer manually to nullptr after deleting it you won’t find it out. That’s the reason why bigger projects either use smart pointer instead of raw pointer or have a lifetime tracker implementation.
    • Example:
      struct Foo {
        int i = 0;
        void foo(){ i++; }
      };
      
      int main() {
        Foo* f;
        f->foo(); // boom
        return 0;
      }
      
  2. Accessing a container without checking for its size or setting up a catch in case of an exception
    • Details: Not all container throw for every not possible access an exception (e.g. std::vector::front).
    • Example:
      int main() {
        std::vector<int> v;
        int d = v.front(); // boom
        return 0;
      }
      
  3. Forgotten virtual destructor in case object gets destructed via base class
    • Details: Just the destructor of the base class will be executed, which leads to a memory leak, cause all other member variables of this object won’t get deleted.
    • Example:
      struct Foo {
        int i = 0;
      };
      
      struct Bar : public Foo {
        int d = 0;
      };
       
      int main() {
        Foo *f = new Bar();
        delete f; // just calls destructor of Foo
        return 0;
      }
      
  4. Forgotten synchronization for multithreaded access on same resources
    • Details: This will lead to a race condition.
    • Example:
      struct Foo {
        int i = 0;
        void foo() {
          for(int d=0; d<5000000; d++) { i++; }
        }
      };
       
      int main() {
        Foo f;
        std::thread t1(&Foo::foo, &f), t2(&Foo::foo, &f);
        t1.join(); t2.join();
        // f.i will be most likely < 10000000
        return 0;
      }
      
  5. Forgotten to release a block mechanism for multithreaded execution
    • Details: This will lead to a deadlock. Prefer the usage of a std::lock_guard over std::mutex::lock/unlock to prevent this.
    • Example:
      struct Foo {
        std::mutex m;
        void foo() { m.lock(); } // the 2nd thread will be blocked
      };
       
      int main() {
        Foo f;
        std::thread t1(&Foo::foo, &f), t2(&Foo::foo, &f);
        t1.join(); t2.join();
        return 0;
      }
      
  6. Passing a ref/raw_ptr of a local variable to a function, which get executed by another thread
    • Details: This leads possibly to an access on an already destructed variable.
    • Example:
      void foo(std::string* s) {
        (*s) += " they lived happily ever after";
      }
      
      int main() {
        std::string s = "Once upon a time ...";
        std::thread t(foo, &s); // boom
        // t.join(); // --> no boom
        return 0;
      }
      
  7. Passing a copy of a big local variable to a function, which get executed by the same thread
    • Details: This problem doesn’t occur for small objects like int cause the size of an pointer address can be even greater.
    • Example:
      void foo(std::string s) {}
      
      int main() {
        std::string s;
        s.resize(5000000000);
        std::fill_n(s.begin(), s.size()/sizeof(char), '0');
        foo(s); // slow
        return 0;
      }
      
  8. Forgotten code documentation (e.g. Doxygen, Follow the link)