RAII - Class Pointers and Scope

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.

Solution

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.