Program Crashing at Destructor

Question

I can't seem to understand why my program runs successfully and then crashes at destructor. Below is my main() source code (which is fairly simple, it sends an array of 5 variables to a class template which creates the appropriate type. I did some research and seem to be missing something that might cause a crash because of an additional call of the destructor? I'm a little fuzzled and it's most likely a simple fix.

main.cpp:

int main() 
{
// using integer data type
int arraya[5] = { 1, 2, 3, 4, 5 };
GenericArray<int> a(arraya, 5);
a.print();

// using float data type
float arrayb[5] = { 1.012, 2.324, 3.141, 4.221, 5.327 };
GenericArray<float> b(arrayb, 5);
b.print();

// using string data type
string arrayc[] = { "Ch1", "Ch2", "Ch3", "Ch4", "Ch5" };
GenericArray<string> c(arrayc, 5);
c.print();
return 0;
}

header file contents:

#ifndef GENERIC_ARRAY_H
#define GENERIC_ARRAY_H

#include<string>
#include<iostream>

template<typename type>
class GenericArray
{
public:
    GenericArray(type array[], int arraySize); // constructor
    ~GenericArray();    // destructor
    void print();       // the print function
    GenericArray(const GenericArray &obj); //copy constructor
private:
    type *ptr; //new pointer of respective type
    int size;
};

template<typename type>//print() function
void GenericArray<type>::print()
{
    for (int index = 0; index < size; index++)
    {
        cout << ptr[index] << " ";
    }
    cout << endl;
}

template<typename type>//Constructor
GenericArray<type>::GenericArray(type array[], int arraySize)
{
    size = arraySize;
    ptr = new type[size];
    ptr = array;
}

template<typename type>//Destructor
GenericArray<type>::~GenericArray()
{
    cout << "Freeing Memory!";
    delete[] ptr;
}

template<typename type>//Copy Constructor
GenericArray<type>::GenericArray(const GenericArray &obj)
{
    *ptr = *obj.ptr;
}

#endif

`delete []` can only be called for pointer allocated with `new` — Amadeus, Feb 10 '17 at 01:24
under the constructor for the class, ptr is allocated memory. — Tony Comito, Feb 10 '17 at 01:24
@Tony Comito Hi, genius. The constructor and the copy constructor are invalid. You have to copy elements of the array instead of assigning a temporary pointer. — Vlad from Moscow, Feb 10 '17 at 01:27
No, in constructor, you allocated `ptr` with `new` then through it away and make ptr to point to where array is pointing too. In c++ array decay to pointer when passing it to functions — Amadeus, Feb 10 '17 at 01:27
@VladfromMoscow So under my constructor should I be dereferencing the array that i'm assigning? — Tony Comito, Feb 10 '17 at 01:28
@Tony Comito You have to copy elements of the array to the allocated memory in the constructor. The same is valid for the copy constructor. Also you need to write the copy assignment operator. — Vlad from Moscow, Feb 10 '17 at 01:30
@Amadeus Ahh, I can see what you're saying! I should be copying elements as Vlad suggested. Should I also implement a copy assignment operator? — Tony Comito, Feb 10 '17 at 01:30
@TonyComito Either define it explicitly or define it as deleted. — Vlad from Moscow, Feb 10 '17 at 01:32

FloHe · Accepted Answer · 2017-02-10T02:16:55.153

-In the print() method:

It isn't safe, that there was memory allocated at memory positions ptr ... (ptr + size - 1), so you might run into a segmentation fault.

-In the constructor:

you allocate memory via new, but then immediately redirect your pointer to point at the same position as array is pointing at. . This means you got a memory leak.

-In the destructor:

As was already mentioned, your program crashes here when the destructor is called, because the delete[] doesn't operate on the memory that was allocated with new, see the constructor remarks.

-In the copy constructor:

There are two problems here. First of all, you can't dereference the lhs-ptr here because there wasn't memory allocated for him. Moreover, if there was memory allocated for ptr, the statement *ptr = *obj.ptr; would just copy the first element of obj.ptr (if there was memory allocated at this position as well) to the first element of ptr.`

Thank you! It works correctly now as I've fixed the issues you brought up and wrote in the copy assignment operator. — Tony Comito, Feb 10 '17 at 02:10
@TonyComito Worth reading: [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) It is sadly under-taught. — user4581301, Feb 10 '17 at 02:12

score 0 · Answer 2 · answered Feb 10 '17 at 01:52

The constructors are defined incorrectly. They shall copy elements of source objects.

For example

#include <algorithm>

//...

template<typename type>//Constructor
GenericArray<type>::GenericArray( const type array[], int arraySize ) 
    : ptr( new type[arraySize] ), size( arraySize )
{
    std::copy( array, array + arraySize, ptr );
}


template<typename type>//Copy Constructor
GenericArray<type>::GenericArray( const GenericArray &obj )
    : ptr( new type[obj.size] ), size( obj.size ), 
{
    std::copy( obj.ptr, obj.ptr + arraySize, ptr );
}

Also you need to define the copy assignment operator.

Thank you! I wrote in the copy assignment operator! instead of using the copy function I manually wrote in a for loop to assign each member. The program works correctly now! I just need to figure out why it's calling the destructor 4 times, hehe. — Tony Comito, Feb 10 '17 at 02:12

Program Crashing at Destructor

2 Answers2