Should I outsource allocation algorithm? (RAII)

Question

Right now my class has a constructor, copy constructor and copy assignment operator which all do the same thing at first (allocating memory). The destructor is deallocating the memory.

class Register
{
public:
    Register()
    {
        _trampoline_address = reinterpret_cast<BYTE*>(VirtualAlloc(nullptr, _trampoline_size, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE));
        if (_trampoline_address == nullptr)
        {
            throw my_exception("Could not allocate memory for trampoline function.");
        }

        //....
    }
    ~Register()
    {
        if (_trampoline_address != nullptr)
        {
            debug(VirtualFree(_trampoline_address, 0, MEM_RELEASE));
        }
    }
    Register(const Register& other)
    {
        _trampoline_address = reinterpret_cast<BYTE*>(VirtualAlloc(nullptr, _trampoline_size, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE));
        if (_trampoline_address == nullptr)
        {
            throw my_exception("Could not allocate memory for trampoline function.");
        }

        //...
    }
    Register& operator= (const Register& other)
    {
        _trampoline_address = reinterpret_cast<BYTE*>(VirtualAlloc(nullptr, _trampoline_size, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE));
        if (_trampoline_address == nullptr)
        {
            throw my_exception("Could not allocate memory for trampoline function.");
        }

        //....
    }

private:
    BYTE* _trampoline_address;
    static const int _trampoline_size = 20;
};

I thought about outsourcing the allocation algorithm because I use it 3 times, but I don't want that other instances of the same class type can access that function.

So what would be the proper solution to allocate memory in 3 functions in a RAII class?

Answer 1

struct Register {
  Register():
    _trampoline_address(allocate())
  {}
  Register(Register const& o):
    Register() // forward to default ctor
  {
    copy_data_from(o);
  }
  ~Register() {
    if (_trampoline_address)
      debug(VirtualFree(_trampoline_address, 0, MEM_RELEASE));
  }
  Register& operator= (const Register& o) {
    if (this != std::addressof(o))
      copy_data_from(o);
    return *this;
  }
private:
  void copy_data_from(Register const& o) {
    Assert(_tranpoline_address);
    // ...
  }
  static BYTE* allocate() {
    return reinterpret_cast<BYTE*>(
      VirtualAlloc(nullptr, _trampoline_size, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE)
    );
  }
  BYTE* _trampoline_address;
  static const/*expr*/ int _trampoline_size = 20;
};

there is only one call to allocate, but I still put it in a static private method because it is messy.

I also wrote copy_data_from, because it will be used twice (once in copy-ctor, once in assignment).

I personally would be tempted to have Register() leave you with an empty buffer, only filled when used. Read functions would have to check for nullptr and allocate() if it was missing: but they have to check for nullptr anyhow. The result is a more efficient move-ctor and move-assign, creating empty arrays (and populating later) is far more efficient, etc.

In this case, allocate() turns out to be more useful. You can even have ensure_allocated(), which initializes _tranpoline_address if nullptr.