7

I've written a RAII wrapper for C function pairs which initialize and release resources and it serves me well for most cases.

#include <GL/glfw.h>
#include <string>
#include <functional>
#include <stdexcept>

template <typename UninitFuncType,
          typename SuccessValueType,
          SuccessValueType successValue>
class RAIIWrapper
{
public:
    template <typename InitFuncType, typename... Args>
    RAIIWrapper(InitFuncType initializer,
                UninitFuncType uninitializer,
                const std::string &errorString,
                const Args&... args) : 
        uninit_func(uninitializer)
    {
        if (successValue != initializer(args...))
            throw std::runtime_error(errorString);
        initialized = true;
    }

    bool isInitialized() const
    {
        return initalized;
    }

    ~RAIIWrapper()
    {
        if (initalized)
            uninit_func();
    }

    // non-copyable
    RAIIWrapper(const RAIIWrapper &) = delete;
    RAIIWrapper& operator=(const RAIIWrapper &) = delete;

private:
    bool initalized = false;
    std::function<UninitFuncType> uninit_func;
};

using GLFWSystem = RAIIWrapper<decltype(glfwTerminate), decltype(GL_TRUE), GL_TRUE>;
using GLFWWindow = RAIIWrapper<decltype(glfwCloseWindow), decltype(GL_TRUE), GL_TRUE>;

int main()
{
    GLFWSystem glfw(glfwInit,
                    glfwTerminate,
                    "Failed to initialize GLFW");
}

However, say when a function returns void like Enter/LeaveCriticalSection I'm not sure how to go about and do it in this class. Should I specialize the class for SuccessValueType = void case? Or something with default template parameter should do?

legends2k
  • 31,634
  • 25
  • 118
  • 222
  • 2
    do you need to have `SuccessValueType` and `successValue` as class template parameters? couldn't they be parameters of the constructor? then you could create two separate constructors... just thinking aloud – Andy Prowl Jan 19 '13 at 21:25
  • Wow, didn't occur to me, lemme try now :) – legends2k Jan 19 '13 at 21:26
  • @AndyProwl: Oh,wait, but that would make my caller site look ugly :( and also when the success value is known @ compile time, passing it @ runtime is unnecessary. – legends2k Jan 19 '13 at 21:26
  • How it would it look ugly? You were going to have to specify the successValue somewhere... – JaredC Jan 19 '13 at 21:28
  • 1
    that would be just one more argument, no? `GL_TRUE` should be enough, its type should be deduced so you won't have to specify it – Andy Prowl Jan 19 '13 at 21:29
  • Why don't you put those init and deinit functions as template parameters too? So you have a single ugly using statement once for every type but a clean instance line. – leemes Jan 19 '13 at 21:30
  • @legends2k: conceptually I would say passing the return value when constructing each instance of the wrapper (i.e. not as a template argument of the class) makes more sense, because each instance of the same wrapper type could invoke a different initialization function and that could have a different success code – Andy Prowl Jan 19 '13 at 21:31
  • An RAII type without a user-defined copy constructor and copy assignment operator? Something smells about that. – Jonathan Wakely Jan 19 '13 at 21:35
  • @JonathanWakely: It's not complete yet, hence it's in the wash @ SO :) – legends2k Jan 19 '13 at 21:35
  • Add deleted copy operations. – Jonathan Wakely Jan 19 '13 at 21:36
  • @JonathanWakely: Happy now? Added 'em :) I omitted them, since they're besides the question's point. – legends2k Jan 19 '13 at 21:39
  • 1
    Overjoyed :) correct copying is always relevant to RAII – Jonathan Wakely Jan 19 '13 at 21:47
  • You initialize uninit wrong. – Yakk - Adam Nevraumont Jan 19 '13 at 21:52

2 Answers2

5

I'd like to note, that

  1. You do not need information on your initialization function in your wrapper class. You only need to know about uninitialization function.

  2. You can create function helpers to instantiate your wrapper.

I came up with the following solution (I liked @ipc exception handling idea)

template <typename UninitF>
struct RAII_wrapper_type
{
    RAII_wrapper_type(UninitF f)
    :_f(f), _empty(false)
    {}
    RAII_wrapper_type(RAII_wrapper_type&& r)
    :_f(r._f), _empty(false)
    {
      r._empty = true;
    }

    RAII_wrapper_type(const RAII_wrapper_type&) = delete;
    void operator=(const RAII_wrapper_type&) = delete;

    ~RAII_wrapper_type()
    {
      if (!_empty) {
        _f();
      }
    }

  private:
    UninitF _f;
    bool _empty; // _empty gets true when _f is `moved out` from the object.
};

template <typename InitF, typename UninitF, typename RType, typename... Args>
RAII_wrapper_type<UninitF> capture(InitF init_f, UninitF uninit_f, RType succ, 
                                   const char* error, Args... args)
{
  if(init_f(args...) != succ) {
    throw std::runtime_error(error);
  }
  return RAII_wrapper_type<UninitF>(uninit_f);
}

template<typename InitF, typename UninitF, typename... Args>
RAII_wrapper_type<UninitF> capture(InitF init_f, UninitF uninit_f, Args... args)
{
  init_f(args...);
  return RAII_wrapper_type<UninitF>(uninit_f);
}

Example:

void t_void_init(int){}
int t_int_init(){ return 1; }
void t_uninit(){}

int main()
{
  auto t1 = capture(t_void_init, t_uninit, 7);
  auto t2 = capture(t_int_init, t_uninit, 0, "some error");
}

Edit

RAII_wrapper_type should have move semantics and we should carefully implement its move constructor to prevent uninit_f from calling several times.

Lol4t0
  • 12,444
  • 4
  • 29
  • 65
  • This is nice and elegant :) I see that a simple function solves the issue here. – legends2k Jan 19 '13 at 23:16
  • Can you make this non-copyable, when I do the usual deletion of copy ctor and assignment operator functions, the compiler complains returning RAII_wrapper_type from capture, which is also a copy. – legends2k Jan 20 '13 at 00:20
  • @legends2k, I've added move issue soultion. – Lol4t0 Jan 20 '13 at 08:59
  • Isn't `_empty` redundant? Can't you just check `_f != nullptr` instead? – legends2k Jan 20 '13 at 12:34
  • @legends2k, if your `UninitF` is actually `C` function pointer, it would be ok, but if it will be a functor, you wouldn't be able neither assign `nullptr` to it, nor compare functor with `nullptr` – Lol4t0 Jan 20 '13 at 13:54
  • I understand functors quite well, thank you. BUT in your implementation above, assigning a `nullptr` to `_f` works, meaning the compiler deduces it as a function pointer. I tried it in both GCC and MSVC (msvc without variadic templates), it deduces the `RAII_wrapper_type`'s template argument as a C-style function pointer. – legends2k Jan 21 '13 at 13:15
  • And this is the reason why I, in the question, have a function as a member variable and use `decltype` to deduce the function type and NOT the function pointer type. – legends2k Jan 21 '13 at 13:17
  • @legends2k, because in an exmaple `t_uninit` _is_ function. If it would be functional object, it wouldn't. – Lol4t0 Jan 21 '13 at 14:06
3

I would seperate the logic of return-Checking and of RAII-Wrapping

template <typename UninitFuncType>
class RAIIWrapper
{
public:
  template <typename InitFuncType, typename... Args>
  RAIIWrapper(InitFuncType fpInitFunc,
              UninitFuncType fpUninitFunc,
              Args&&... args)
    : fpUninit(std::move(fpUninitFunc))
  {
    static_assert(std::is_void<decltype(fpInitFunc(args...))>::value, "ignored return value");
    fpInitFunc(std::forward<Args>(args)...);
  }

  bool isInitialized() const { return true; } // false is impossible in your implementation

  ~RAIIWrapper() { fpUninit(); } // won't be called if constructor throws

private:
  UninitFuncType fpUninit; // avoid overhead of std::function not needed
};

template <typename InitFuncType, typename UninitFuncType, typename... Args>
RAIIWrapper<UninitFuncType>
raiiWrapper(InitFuncType fpInitFunc,
            UninitFuncType fpUninitFunc,
            Args&&... args)
{
  return RAIIWrapper<typename std::decay<UninitFuncType>::type>
    (std::move(fpInitFunc), std::move(fpUninitFunc), std::forward<Args>(args)...);
}

template <typename InitFuncType, typename SuccessValueType>
struct ReturnChecker {
  InitFuncType func;
  SuccessValueType success;
  const char *errorString;
  ReturnChecker(InitFuncType func,
                SuccessValueType success,
                const char *errorString)
    : func(std::move(func)), success(std::move(success)), errorString(errorString) {}

  template <typename... Args>
  void operator()(Args&&... args)
  {
    if (func(std::forward<Args>(args)...) != success)
      throw std::runtime_error(errorString);
  }
};
template <typename InitFuncType, typename SuccessValueType,
          typename Ret = ReturnChecker<InitFuncType, SuccessValueType> >
Ret checkReturn(InitFuncType func, SuccessValueType success, const char *errorString)
{
  return Ret{func, success, errorString};
}

I also added functions to allow type deduction. Here is how to use it:

auto _ = raiiWrapper(checkReturn(glfwInit, GL_TRUE, "Failed to initialize GLFW"),
                     glfwTerminate);

Since having a function object that has a non-void return value causes the static_assert to fail, the following is impossible:

raiiWrapper(glfwInit, glfwTerminate); // fails compiling

If you really want to ignore it, you can add an ignoreReturn function object. Also note that the return code checking can be as sophisticated as you want (like success has to be an even number) since you can write your own return code checker.

legends2k
  • 31,634
  • 25
  • 118
  • 222
ipc
  • 8,045
  • 29
  • 33
  • +1 for showing me that `function<>` isn't required and also for a good answer, I like this solution, but let's give it more time, since I still feel this could be done without decoupling the 2 operations. – legends2k Jan 19 '13 at 21:43
  • Are you sure that function is not required? I tried compiling, clang throws _error: data member instantiated with function type 'void ()'_. I think you intend it to be used as a function pointer i.e. UninitFuncType*. I didn't want that, hence I used `function`. – legends2k Jan 19 '13 at 21:46
  • 1
    Do some `std::decay` to fix that. – Yakk - Adam Nevraumont Jan 19 '13 at 21:50
  • @ipc: The functor ReturnChecker's operator() should take variable arguments; edited the answer for this. – legends2k Jan 19 '13 at 22:46
  • That could easily fixed, but I like the solution from @Lol4t0 more, so use them ;) – ipc Jan 19 '13 at 22:54
  • @ipc: Yes, I up voted it too :) But I don't just copy-pasting code, I try to it & that's why I come to SO. Could you explain the `static_assert` in your code? I.e. the functor's operator() will always be returning `void`, then why do you check it for `is_void`? – legends2k Jan 19 '13 at 23:09
  • @ipc What is more queer is that I don't see the static_assert's message flagged ever by the compiler, but ideally I should see it all the time, since the return type is void all the time. – legends2k Jan 19 '13 at 23:12
  • The `static_assert` checks whether the return value is void, if it's not, then it fails. This is, because otherwise, `raiiWrapper(glfwInit, glfwTerminate);` would be valid code too. IMHO ignoring (aka forgetting to check it) such a return value is mostly an error. – ipc Jan 19 '13 at 23:28
  • @ipc: I understand that, I guess you didn't read my comments properly. The functor ReturnChecker's operator() will always be returning `void`, which is actually the input for the RAIIWrapper ctor and there by to `static_assert`, then why do you check for it's voidness? Yes, it makes sense in the case where the caller directly calls raiiWrapper circumventing the functor. – legends2k Jan 19 '13 at 23:49