c++memorysmart-pointersbad-alloc

would memory be leaked if on allocation failed in this code?


so for my homework I am not allowed to use std::smart_ptr however I am allowed to implement it myself and make any changes I want

so this is what I did

#ifndef SMART_PTR_H_
#define SMART_PTR_H_

template<class T>
class smart_ptr {
    T* data;
public: typedef T element_type;
    explicit smart_ptr(T* ptr = NULL)
   : data(ptr) {}
    ~smart_ptr() {  delete data;    }
    T& operator*() const {  return *data; }
    smart_ptr& operator=(smart_ptr<T>&);
    };
template<class T>
smart_ptr<T>& smart_ptr<T>::operator=(smart_ptr<T>& ptr)
{
    delete this->data;
    T* new_data = new T(*ptr);
    this->data =new_data;
    return *this;
}
#endif

so my question is , for a code like this:

template <class T>
SortedList<T>::SortedList(const SortedList<T>& list):
  data(new smart_ptr<T>[list.max_size])
 ,size(list.size)
 ,max_size(list.max_size)
 {
     for (int i = 0; i < size; i++)
     {
        data[i]=list.data[i];// use of operator= of smart_ptr
     }
 }

so IF a new threw std::bad_alloc would there be a memory allocation or would the destructor of smart_ptr take care of everything?


Solution

  • so IF a new threw std::bad_alloc would there be a memory allocation

    If new throws, then that new will not have allocated memory. However, it is possible for the constructor to have made their own allocations, and a poorly implemented constructor can leak those allocations if the constructor throws.

    You have a much worse bug than a mere memory leak:

    delete this->data;
    T* new_data = new T(*ptr);
    

    If new throws, then this->data will be left with an invalid pointer. Within the destructor, that invalid pointer will be deleted and the behaviour of the program is undefined.

    which means that I should probably do the delete after the new ?

    That would be much better, but it would still have potential for memory leak in case the destructor throws.

    You should probably transfer the ownership of either smart pointer into a third, local smart pointer temporarily.


    but the thing is if the new throws what will happen to all the new that succeeded in the for loop?

    SortedList<T>::SortedList(const SortedList<T>& list):
        data(new smart_ptr<T>[list.max_size])
    

    The new[] array is owned by SortedList::data. If that is a smart pointer, then it should be taken care of within its destructor. If it is a bare pointer, then the pointed array as well as the smart pointers within the array will leak.

    Note that since you allocated an array, the shown smart_ptr::~smart_ptr won't do the correct thing since it doesn't use delete[].

    actually SortedList::~SortedList does delete [] data, would that be enough?

    No. If the constructor of SortedList throws, then its destructor won't be called.