2

I have the following code:

src/main.cpp:

#include <include/array.hpp>

int main() {
    int temp[] = {1, 2, 3, 4, 5};
    sdizo::array<int> a(temp, 5);
    print_array(a);
    return 0;
}

include/array.hpp

#pragma once
#include <string.h>
#include <iostream>

namespace sdizo {

template <class T>
class array {
private:
    T* data;
    int length;

public:
    array();
    array(T* d, int len);
    ~array();
    T* begin();
    T* end();
};
template <typename T>
array<T>::array() {
    data = nullptr;
    length = 0;
}

template <typename T>
array<T>::array(T* d, int len) {
    length = len;
    this->data = new T[len];
    memcpy(this->data, d, sizeof(d[0]) * len);
}
template <typename T>
array<T>::~array() {
    delete[] this->data;
}
template <typename T>
T* array<T>::begin() {
    return &data[0];
}
template <typename T>
T* array<T>::end() {
    return &data[length];
}

template <typename T>
void print_array(array<T> ar) {
    for (auto x : ar) {
        std::cout << x << " ";
    }
    std::cout << '\n';
}
}

Compiling it with: g++ src/*.cpp -I./ -std=c++1z -g -o bin/sdizo1

Now running it gives the following error:

enter image description here

I've noticed that when I didn't print the array, then no errors occured. I thought about passing the array by reference, cause for now it would create a local copy and maybe the error was caused by calling the destructor twice. So void print_array(array<T> &ar) instead void print_array(array<T> ar) worked.

The problem is gone now, but why is that. I know that probably the ~array() was called twice, or something like that, on the same memory location, but why. Isn't the C++ compiler smart enough to detect those kind of things? Or should I always pass by reference, or is there a way to pass by value and not receive those kind of errors with a structure with custom made destructor?

I know that it will be probably boring for many ppl smarter than me, but hey, he that nothing questioneth, nothing learneth.

minecraftplayer1234
  • 2,127
  • 4
  • 27
  • 57
  • You cannot use `memcpy` with non-[POD](http://en.cppreference.com/w/cpp/concept/PODType) (plain old data) types. Your container will therefore not work with non-POD types. – François Andrieux Aug 22 '17 at 18:33
  • "The problem is gone now, but why is that." -- good instinct. Problems that go away by themselves come back by themselves. If you don't know what caused it you haven't fixed it. – Pete Becker Aug 22 '17 at 18:46

1 Answers1

5

See rule of three/five/zero. You have a class with a raw owning pointer. Your default copy constructor and copy assignment operator will not do what you might expect, they only copy the pointer. Two or more instances have a pointer to the same data and each deletes it.

Edit : To clarify, the reason you do not observe a problem when you change print_array to take it's argument by reference is that you would not be using the defective copy constructor or copy assignment operator. This is not a problem with print_array, it's a defect in the array class that must be corrected.

François Andrieux
  • 28,148
  • 6
  • 56
  • 87
  • So as the `Rule of Three` says, I should write my own copy constructor and copy assignment operator? – minecraftplayer1234 Aug 22 '17 at 18:56
  • 1
    @Frynio Yes, your pretty much have to. If you require a destructor, you almost always require at least a copy constructor and copy assignment operator. Other cases may call for deleting them instead. Keep in mind that if you can redesign your class to not require a destructor, you are likely to remove the need for a copy constructor and copy assignment (rule of zero). Consider using existing standard containers instead. – François Andrieux Aug 22 '17 at 18:57
  • Making a project for algorithms and data structures class, have to not use STL containers/algorithms :D – minecraftplayer1234 Aug 22 '17 at 19:27