Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Aim:

Use C++0x features to make function interposition safer. The problem is that it's easy to make a typo when wrapping and interposing on functions.

Prototype Implementation:

#define MAKE_WRAPPER(x) static const wrapper<decltype(::x), ::x> x(#x)

namespace {
  template<typename Sig, Sig& S>
  struct wrapper;

  template<typename Ret, typename... Args, Ret(&P)(Args...)>
  struct wrapper<Ret(Args...), P> {
         typedef Ret(*real_func)(Args...);
         real_func f;
         wrapper(const std::string& sym) : f(NULL) {    
                dlerror();
                void *ptr = dlsym(RTLD_NEXT, sym.c_str());
                f = (real_func)ptr;
                if (NULL == f) {
                  std::cerr << "dlsym(): " << dlerror() << std::endl;
                }
                std::cout << "constructing wrapper: " << this << " for: " << sym << "(at " << ptr << ")" << std::endl;
         }

         Ret operator()(const Args&... args) const {
                return f(args...);
         }
  };
}

Example usage:

extern "C" {
  void glXSwapBuffers(Display *dpy, GLXDrawable drawable) {
         MAKE_WRAPPER(glXSwapBuffers);
         // Do stuff here before making the call to the wrapper
         glXSwapBuffers(dpy, drawable);
  }
}

Questions:

  1. Is it worth the effort?
  2. How can it be improved?
share|improve this question
add comment

1 Answer

up vote 3 down vote accepted

[ I'm assuming your example is meant to return x(dpy, drawable); rather than call itself recursively. ]

  1. It's worth it if you have a use for it I guess.

  2. Printing to std::cerr is not error handling, it's error logging. Pick a real error handling strategy and stick to it. Similarly is printing to std::cout necessary? I also believe that the fewer library calls there are in the constructor, the fewer risks of your wrapper calling itself recursively during static initialization of x, which will be disastrous.

Posix recommends the following way to cast when dealing with function types with dlsym:

*(void**)&f = dlsym(RTLD_NEXT, sym.c_str());

operator() can use perfect forwarding:

Ret
operator()(Args... args)
{
    f(std::forward<Args>(args));
}

The wrapping will cost two moves or one copy + one move per argument passed by value and one forward for reference types. It is transparent to the client; he or she won't need to use std::move unless the original function would already have him required to. Notice that the first copy or move of a by-value argument must be paid no matter what; the real opportunity cost of the wrapper is the (potential) move that comes after that, to pass the argument to the real function.

By comparison using Args const&... breaks if the wrapped function is void foo(T&&);, due to reference collapsing rules the signature will be void operator()(T&);

Perhaps the macro should take the name of the variable as an argument; but that may not be necessary depending on your planned usage.

Minor pet peeve: meaningful names. I would have called the constructor parameter symbol. The time it takes to type three more characters each time is simply negligible compared to the time it takes to write and maintain code. function_type instead of real_func possibly, too.

share|improve this answer
    
+1, thanks there's quite a few interesting things for me to think about in your answer. –  Flexo Aug 13 '11 at 0:28
    
(also the call to glXSwapBuffers in the example is correct, in that it isn't recursive. The name of the variable is derived from the macro parameter x) –  Flexo Aug 13 '11 at 0:33
    
One of the things I was trying to work out was if I could make the constructor deduce the name of the symbol (by looking at __FUNC__ and friends) rather than having to pass it in, but I think the MAKE_WRAPPER macro makes mostly that redundant. –  Flexo Aug 13 '11 at 0:35
    
@awoodland Looks like I forgot that x was the macro parameter by the time I got to reading the variable name, heh. __func__ does get you the name of the function but as a string. So yeah, since you need the function for decltype that won't help. –  Luc Danton Aug 13 '11 at 0:46
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.