Monday, May 5, 2014

inline, static, extern functions and namespaces

How safe do you think it is to write this piece of code in your .cpp file ?


file.cpp
struct Point {
    double *px;
    double *py;

    Point (double x, double y) {
        px = new double(x);
        py = new double(y);
    }
    ~Point() {
        delete x;
        delete y;
    }

    // other members ...
};

Read more why this is the most dangerous piece of code you can write in your .cpp, and it is more likely to be spotted after releasing the project...

I am not talking about new throwing an exception as it runs out of memory.

The real danger here exists because somebody else, in another part of the program, or in a header, might create another struct or class with the same name -- Point.



file2.cpp

class Point {
    float pt[3];

   public:
    Point (float x, float y, float z)
        : pt[0](x), pt[1](y), pt_z[2]
    {
    }
    
    ~Point() {
    }

    // other members ...
};

What's the problem with this setting ?

You'll wonder:
- "But, each Point is static in its own .cpp file. There is no overlap between the files".
- "The compiler doesn't complain anyway".
- "In C, it is possible to have different structs with the same name but different definition in separate .c files"

The problem is not in the data structure. The problem originates from the inline functions.

Consider the following example:

a.cpp 
#include <vector>
...
vector<int> v;


b.cpp
#include <vector>
...
vector<int> w;


Now, when you compile this, the includes are essentially copied into the file-to-be-compiled, resulting into:


a.cpp
template <class T>  class vector {
       void push_back( const T& x ) { ... }
       ...
};
...
vector<int> v;


b.cpp
template <class T>  class vector {
       void push_back( const T& x ) { ... }
       ...
};
...
vector<int> w;

So, temporarily, there is a copy of vector's code in each .cpp module. Of course this would result to code bloat in the binary file, so, when the linker kicks in, it removes duplicates of the inline functions. Any inline function whose prototype matches is removed, and only a single copy is kept.

This is what will happen to your Point classes, whose inline destructors' prototype match. It is also likely that there are other inline functions that their prototypes will match as well. Perhaps a draw(), or a shift() member function.

The way to tell the linker that Point classes are different, is that they have a different name. Or, more practivally, to put them in a different namespace.


file.cpp
namespace A {
  struct Point {
      double *px;
      double *py;

      Point (double x, double y) ...
      ~Point() {...}

      // other members ...
  };
} // namespace A


file2.cpp
namespace B {
  class Point {
      float pt[3];

    public:
      Point (float x, float y, float z) ...    
      ~Point() {...}

       // other members ...
  };
} // namespace B


If no name is suitable for your namespace, the best thing you can do is use an unnamed namespace, instead, like so:




file.cpp
namespace {  // unnamed namespace
  struct Point {
      double *px;
      double *py;

      Point (double x, double y) ...
      ~Point() {...}

      // other members ...
  };
} // namespace 


file2.cpp
namespace {  // unnamed namespace
  class Point {
      float pt[3];

    public:
      Point (float x, float y, float z) ...    
      ~Point() {...}

       // other members ...
  };
} // namespace


This way, no aliasing will happen. Every translation unit (.cpp file) has its own namespace. 
Never declare things inside an unnamed namespace in a .hpp file though!


---------------- 

The same problem (and solution) holds for plain inline functions:

file1.cpp
inline void do_something() {
    // does something
}


file2.cpp
inline void do_something() {
    // does something different
}



To avoid aliasing, define them inside a namespace:


file1.cpp
namespace {
  inline void do_something() {
      // does something
  }

}

file2.cpp
namespace {
  inline void do_something() {
      // does something different
  }

}

No comments:

Post a Comment