HEAP CORRUPTION DETECTED : after normal block ()

Question

So, I've defined template class and then i tried to overload some operators.

 template <typename  T> class Set
    {
    public:
        Set(void);
        Set(Set&);
        ~Set(void);
        bool contains(T elem);
        bool add(T elem);
        bool remove(T elem);
        bool add(T* tab, int size);
        T* getSet();
        int size();
        Set<T> &operator+(Set<T> &snd);
        Set<T> &operator-(Set<T> &snd);
    private:
        T *elements;
        int numOfElem;
    };

When I try to add element to the Set by add method everything works fine.

template<typename T>
    bool Set<T>::add(T elem)
    {
        bool found = false;
        for(int i =0; !found && i<numOfElem; i++){
            if(elem == elements[i]) found = true;
        }
        if( !found ){
            numOfElem++;
            T* tmp = new T[numOfElem];
            for(int i =0;  i<numOfElem-1; i++){
                tmp[i] = elements[i];
            }
            tmp[numOfElem-1] = elem;
            delete[] elements;
            elements = tmp;
        }
        return !found;
    }

    template<typename T>
    bool Set<T>::add(T* myArray, int size)
    {
        bool result = false;
        for(int i =0;  i<size; i++){
            add(myArray[i]);
        }
        return result;
    }
template<typename T> 
Set<T>& Set<T>::operator+(Set<T> &snd)
{
    Set *temp = new Set(*this);
    temp->add(snd.getSet(), snd.size());
    return *temp;
}
template<typename T> 
void Set<T>::operator=(Set<T> &snd)
{
    numOfElem = snd.numOfElem;
    elements = new T[numOfElem];
    for(int i =0; i < numOfElem; i++){
        elements[i] = snd.elements[i];
    }
}

template<typename T>
int Set<T>::size()
{
    return numOfElem;
}
template<typename T> 
T* Set<T>::getSet()
{
    return elements;
}
template<typename T>
Set<T>::Set()
{
  numOfElem = 0;
  elements = nullptr;
}

template<typename T>
Set<T>::Set(Set& old)
{
  numOfElem = old.numOfElem;
  elements = new T(numOfElem);
  for(int i = 0; i< numOfElem; i++){
      elements[i] = old.elements[i];
  }

}

template<typename T>
Set<T>::~Set()
{
  numOfElem = 0;
  delete[] elements;
  elements = nullptr;
}

But if I use + operator instead (adding two separate sets) the error occurs while trying to delete the array (15 Line). Any ideas?

int main(){
    Set <char> set1, set2, set3;
    char tab[] = {'a','d','f','g'} ;
    set1.add(tab, 4);
    char tab2[] = {'a','d','x','y','z'} ;
    set2.add(tab2,5);
    set3= set1+set2;
}

Returning a reference to a local object is never wise. You should have listened to your compiler's warnings (`return temp` from `operator+`). Change the signature to `template Set Set::operator+(Set &snd)` and nobody gets hurt. — IInspectable, Jan 10 '15 at 01:59
@PiotrSzymczyk _"Ok, good point, yet it doesn't solve the problem."_ Well, **you** are expected to give a [MCVE](http://stackoverflow.com/help/mcve) for your problems, instead of declining suggestions, and awaiting other users to debug your code! Unfortunately I've been close voting your question for a different reason, but in reality you're asking about a duplicate: [Can a local variable's memory be accessed outside its scope?](http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope). I should have bombed out your question for this in 1st place. — πάντα ῥεῖ, Jan 10 '15 at 02:20
I'd guess double delete. If you have a raw pointer you need to consider the rule of three. I only see two. http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — Retired Ninja, Jan 10 '15 at 02:30
Sorry that's my first post here, i've updated the code (MCVE) to show that the problem does exist. Copy assignment operator aslo didn't help. — Piotr Szymczyk, Jan 10 '15 at 02:58
Did you want to use brackets `[]` here? `elements = new T(numOfElem);` It would help anyone trying to actually run your code if you put all the code in one block so it can be copy/pasted easily, and if you used the code formatting button, not the snippets button to format the code. — Retired Ninja, Jan 10 '15 at 03:40
`operator=` leaks memory, should return a reference to self (`Set&`), and take a const ref parameter (`const Set&`). See [Copy assignment operator](http://en.cppreference.com/w/cpp/language/as_operator) for reference. — IInspectable, Jan 10 '15 at 13:01

Marian Spanik · Accepted Answer · 2015-01-10T13:30:53.583

0

You have a mistake in your copy constructor:

elements = new T(numOfElem);

It should be

elements = new T[numOfElem];

By writing new T(numOfElem); you allocate only one variable with its value initialized to numOfEllem.

Use a std::vector instead of the array and you will avoid such problems.

Your code is also leaking a memory in the addition operator:

template<typename T> 
Set<T>& Set<T>::operator+(Set<T> &snd)
{
    Set *temp = new Set(*this);
    temp->add(snd.getSet(), snd.size());
    return *temp;
}

You are allocating a memory and you never delete it so if you call that function too often you program may run out of its virtual memory and will crash with the uncaught std::bad_alloc exception. Change the function to this:

template<typename T>
Set<T> Set<T>::operator+(Set<T> &snd)
{
    Set temp(*this);
    temp.add(snd.getSet(), snd.size());
    return temp;
}

edited Jan 10 '15 at 13:30

answered Jan 10 '15 at 13:01

Marian Spanik

1,079
1
12
18

Swapping assumes an OS with virtual memory backed by the file system. While common, this is not strictly mandatory. Assuming virtual memory, an application will usually run out of address space, not memory. Your conclusions are equally wrong: Swapping does not affect system stability. And leaking memory does not cause swapping: That memory will eventually get paged out, and since you hold no reference to it, it will never be paged in again. You may want to revise (or remove) that paragraph altogether. – IInspectable Jan 10 '15 at 13:10
I removed that part with swapping as it used inaccurate terminology. I meant that when you run that addition in infinite loop in a 64-bit program, a size of your physical operating memory is much smaller that the size of the address space so the OS will have to swap huge amount of pages to the disk and the OS may become unstable during that (I did test it on Windows 7 and I had to reset a computer). The program will eventually crash when the swap on the disk reaches its maximum capacity or the program's virtual memory becomes full, but it will take really long time, unless you have a SSD disk. – Marian Spanik Jan 10 '15 at 13:44

HEAP CORRUPTION DETECTED : after normal block ()

1 Answers1