Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Coding style conformance

...

Code Block
bgColor#FFcccc
langcpp
class Handle {
  class Body *Implimpl;  // Declaration of a pointer to an incomplete class
public:
  ~Handle() { delete Implimpl; } // Deletion of pointer to an incomplete class
  // ...
};

...

In this compliant solution, the deletion of Impl impl is moved to a part of the code where Body is defined:

Code Block
bgColor#ccccff
langcpp
class Handle {
  class Body *Implimpl;  // Declaration of a pointer to an incomplete class
public:
  ~Handle();
  // ...
};

// Elsewhere
class Body { /* ... */ };
 
Handle::~Handle() {
  delete Implimpl;
}

Compliant Solution (std::shared_ptr)

In this compliant solution, a std::shared_ptr is used to own the memory to Implimpl. Note that a std::shared_ptr is capable of referring to an incomplete type, but a std::unique_ptr is not.

Code Block
bgColor#ccccff
langcpp
#include <memory>
 
class Handle {
  std::shared_ptr<class Body> Implimpl;
  public:
    Handle();
    ~Handle() {}
    // ...
};

...

Pointer downcasting (casting a pointer to a base class into a pointer to a derived class) may require adjusting the address of the pointer by a fixed amount that can be determined only when the layout of the class inheritance structure is known. In this noncompliant code example, f() retrieves a polymorphic pointer of complete type B from getDget_d(). That pointer is then cast to a pointer of incomplete type D before being passed to g(). Casting to a pointer to the derived class may fail to properly adjust the resulting pointer, causing undefined behavior when the pointer is dereferenced by calling d->doSomething>do_something().

Code Block
bgColor#FFcccc
langcpp
// File1.h
class B {
protected:
  double d;
public:
  B() : d(1.0) {}
};
 
// File2.h
void g(class D *);
class B *getDget_d(); // Returns a D object

// File1.cpp
#include "File1.h"
#include "File2.h"

void f() {
  B *v = getDget_d();
  g(reinterpret_cast<class D *>(v));
}
 
// File2.cpp
#include "File2.h"
#include "File1.h"
#include <iostream>

class Hah {
protected:
  short s;
public:
  Hah() : s(12) {}
};

class D : public Hah, public B {
  float f;
public:
  D() : Hah(), B(), f(1.2f) {}
  void doSomethingdo_something() { std::cout << "f: " << f << ", d: " << d << ", s: " << s << std::endl; }
};

void g(D *d) {
  d->doSomething>do_something();
}

B *getDget_d() {
  return new D;
}

Implementation Details

When compiled with Clang 3.58, the noncompliant code example prints the following:

...

Similarly, unexpected values are printed when the example is run in Microsoft Visual Studio 2013 2015 and GCC 46.91.0.

Compliant Solution

This compliant solution assumes that the intent is to hide implementation details by using incomplete class types. Instead of requiring a D * to be passed to g(), it expects a B * type:

Code Block
bgColor#ccccff
langcpp
// File1.h
class B {
protected:
  double d;
public:
  B() : d(1.0) {}
};
 
// File2.h
void g(class B *); // Accepts a B object, expects a D object
class B *getDget_d(); // Returns a D object

// File1.cpp
#include "File1.h"
#include "File2.h"

void f() {
  B *v = getDget_d();
  g(v);
}
 
// File2.cpp
#include "File2.h"
#include "File1.h"
#include <iostream>

class Hah {
protected:
  short s;
public:
  Hah() : s(12) {}
};

class D : public Hah, public B {
  float f;
public:
  D() : Hah(), B(), f(1.2f) {}
  void doSomethingdo_something() { std::cout << "f: " << f << ", d: " << d << ", s: " << s << std::endl; }
};

void g(B *d) {
  D *t = static_cast<D *>(d);
  if (t) {
    t->doSomething>do_something();
  } else {
    // Handle error
  }
}

B *getDget_d() {
  return new D;
}

Risk Assessment

...