2

I want to gain a better understanding of how to implement the RAII idiom with my classes, through an example: What the recommended method is for ensuring pointers are free()'d properly in my class?

I have a class which should exist for the duration of the program. In the spirit of RAII and because I need to pass a reference to this class to other classes, I am holding it in a shared_ptr (not sure it actually needs to be held in a shared_ptr, but for fun, it is).

In the class ctor, I use 2 buffers (pointers) and then loop multiple times malloc()'ing, using the buffer and then free()'ing. The dtor should contain failsafe code to free the buffers, in the event of mishap.

The only way the dtor can see the buffers is if I declare them as class variables, however they are only used in the class ctor.

Example:

class Input
{
private:
    PSOMETYPE buffer1;
public:
    Input();
    ~Input();
}

Input::Input() : buffer1(NULL)
{
    for(blahblah)
    {
        buffer1 = (PSOMETYPE)malloc(sizeof(SOMETYPE));
        // Do work w/buffer1
        if(buffer1 != NULL) { free(buffer1); buffer1 = NULL }
    }
}

Input::~Input()
{
    if(buffer1 != NULL) { free(buffer1); buffer1 = NULL }
}

Considering I only use the buffer in the ctor, does it make sense to declare it as a private class variable? If I declare it in the scope of the ctor, the dtor will have no knowledge as to what it is to free.

I know this is a trivial example, and honestly I could implement this as easily forgetting about using a smart pointer to reference my class and having a blank dtor, just free()'ing as I'm doing inside the loop. I have no mentor or schooling, and I'm uncertain of when the RAII idiom should be followed.

Lokked
  • 63
  • 9
  • 3
    This is a strange mix of C and C++ ideas... Is there a reason you're using malloc instead of new, or even better, STL templated containers? Those fit much better with the concept of RAII – tmpearce Mar 13 '12 at 16:25
  • @tmpearce: This is due to my inexperience and using MSDN as a learning resource. I appreciate the references for alternatives/improvements. – Lokked Mar 13 '12 at 16:53

1 Answers1

6

The spirit of RAII would be to use a local object to manage the locally allocated object, rather than artificially tying its lifetime to the object being constructed:

class Input
{
    // no pointer at all, if it's only needed in the constructor
public:
    Input();
    // no explicit destructor, since there's nothing to explicitly destroy
};

Input::Input()
{
    for(blahblah)
    {
        std::unique_ptr<SOMETYPE> buffer1(new SOMETYPE);

        // or, unless SOMETYPE is huge, create a local object instead:
        SOMETYPE buffer1;

        // Do work w/buffer1
    }   // memory released automatically here
}

You should only ever have to use delete (or free, or whatever) yourself if you're writing a class whose purpose is to manage that resource - and usually there's already a standard class (such as a smart pointer or a container) that does what you want.

When you do need to write your own management class, always remember the Rule of Three: if your destructor deletes something, then the default copying behaviour of the class will almost certainly cause a double delete, so you need to declare a copy constructor and copy-assignment operator to prevent that. For example, with your class I could write the following incorrect code:

{
    Input i1;     // allocates a buffer, holds a pointer to it
    Input i2(i1); // copies the pointer to the same buffer
}                 // BOOM! destroys both objects, freeing the buffer twice

The simplest way to prevent this is to delete the copy operations, so code like that will fail to compile:

class Input {
    Input(Input const&) = delete;    // no copy constructor
    void operator=(Input) = delete;  // no copy assignment
};

Older compilers may not support = delete; in which case you can get almost the same effect by declare them privately without = delete, and not implementing them.

Community
  • 1
  • 1
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • Thank you. This makes sense to me. Your first sentence summed things up perfectly. In your example of incorrect code, would an exception be thrown due to copying a unique_ptr? I would definitely need to implement the copy ctor and copy-assignment operator. Thanks again! – Lokked Mar 13 '12 at 16:41
  • @Lokked: If the class contained a `unique_ptr`, then you'd get a compiler error if you tried to copy it. You could instead use `shared_ptr` to get safe pointer copying, or embed an object rather than a pointer to get object copying. In my example, the `unique_ptr` wouldn't be a member at all, since it's only needed within the constructor. – Mike Seymour Mar 13 '12 at 16:44