views:

165

answers:

5

I'm working on a section of code that has many possible failure points which cause it to exit the function early. The libraries I'm interacting with require that C-style arrays be passed to the functions. So, instead of calling delete on the arrays at every exit point, I'm doing this:

void SomeFunction(int arrayLength)
{
   shared_ptr<char> raiiArray(new char[arrayLength]);
   pArray = raiiArray.get();

   if(SomeFunctionThatRequiresCArray(pArray) == FAILED) { return; }

   //etc.
}

I wanted to use unique_ptr, but my current compiler doesn't support it and the reference count overhead doesn't really matter in this case.

I'm just wondering if anyone has any thoughts on this practice when interfacing with legacy code.

UPDATE I completely forgot about the shared_ptr calling delete instead of delete []. I just saw no memory leaks and decided to go with it. Didn't even think to use a vector. Since I've been delving into new (for me) C++ lately I'm thinking I've got a case of the "If the only tool you have is a hammer, everything looks like a nail." syndrome. Thanks for the feedback.

UPDATE2 I figured I'd change the question and provide an answer to make it a little more valuable to someone making the same mistake I did. Although there are alternatives like scoped_array, shared_array and vector, you can use a shared_ptr to manage scope of an array (but after this I have no idea why I would want to):

template <typename T>
    class ArrayDeleter
    {
    public:
        void operator () (T* d) const
        {
            delete [] d;
        }
    };

void SomeFunction(int arrayLength)
    {
       shared_ptr<char> raiiArray(new char[arrayLength], ArrayDeleter<char>());
       pArray = raiiArray.get();

       if(SomeFunctionThatRequiresCArray(pArray) == FAILED) { return; }

       //etc.
    }
+5  A: 

There's boost::scoped_ptr for this.

Nikolai N Fetissov
scoped_array in this case
Luther Blissett
Yes, sure, they are both there. Thanks.
Nikolai N Fetissov
+13  A: 

Use boost::scoped_array, or even better std::vector if you are dealing with an array.

Nemanja Trifunovic
+8  A: 

Do not use shared_ptr or scoped_ptr to hold pointers to dynamically allocated arrays. shared_ptr and scoped_ptr use delete ptr; to clean-up when the pointer is no longer referenced/goes out of scope, which invoked undefined behaviour on a dynamically allocated array. Instead, use shared_array or scoped_array, which correctly use delete[] ptr; when destructing.

To answer your question, if you are not going to pass the smart pointer around, use scoped_array, as it has less overhead than shared_array.

Alternatively, use std::vector as the array storage (vectors have guaranteed contiguous memory allocation).

Jon Benedicto
+3  A: 

This

shared_ptr<char*> raiiArray(new char[arrayLength]);

is not a good practice, but causes undefined behaviour, as you allocate with operator new[], but shared_ptr uses operator delete to free the memory. The right thing to use is boost::shared_array or add a custom deleter.

jpalecek
+6  A: 

I highly recommend simply using a std::vector. Elements in vectors are allocated on the heap, and will be deleted when the vector goes out of scope, wherever you exit the function.

In order to pass a vector to legacy code requiring C-style arrays, simply pass &vectorName[0]. The elements are guaranteed to be contiguous in memory.

Justin Ardini
AshleysBrain