1

I have a class with pointer to char. I am wondering how is it possible that copy construction is possible without explicitly defining copy constructor?

I assume because the object was already allocated, but if so, then why would someone need copy constructors in the first place?

#include <iostream>
#include <cstring>

class Data {
public:
    explicit Data(const char* newData)
        :size{strlen(newData)},
         data{size ? new char[size] : nullptr}
    {
        std::memcpy(data, newData, size);
    }

    friend std::ostream& operator<<(std::ostream& ost, const Data& rhs) {
        for (std::size_t i = 0; i < rhs.size; i++) {
            std::cout << rhs.data[i];
        }
        ost << "\n";
        return ost;
    }

~Data() {
    delete[] data;
}

private:
    std::size_t size;
    char* data;
};

int main() {
    Data data1{"data1"};
    Data data2{data1}; // copy constructor
    std::cout << data1;
    std::cout << data2;

    return 0;
}

Output:

data1
data1

Shouldn't the copy constructor look like that? I am often seeing such examples. But since the default constructor already did that, then when do I actually need to define copy ctor?

Data(const Data& other) {
    if (this != &other) {
        size = other.size;
        data = new char[size];
        std::memcpy(data, other.data, size);
    }
}

By the way, I realize the code introduces some maul practices (e.g. using new instead of smart ptrs, not using string in the first place, etc. - but it's just an exercise).

weno
  • 804
  • 8
  • 17
  • 2
    You don't need to check `this != &other` in the copy constructor because an object that is not already constructed cannot be the same as the given object (you can't pass what's not already existing). This check is useful in the copy-assignment operator though :) – Fareanor Jan 09 '20 at 09:40

1 Answers1

3

Since C++11, the generation of the implicitly-defined copy constructor is deprecated (for a good reason) if a class has a user-defined destructor, but it is still generated. That copy constructor will just copy size and data, making a shallow copy, not a deep one. This a road to disaster, because data will then be deleted multiple times, when the original object and its copies are destroyed.

For example, if you run your original code under Valgrind, you'll see the following report:

==9908== HEAP SUMMARY:
==9908==     in use at exit: 0 bytes in 0 blocks
==9908==   total heap usage: 3 allocs, 4 frees, 73,733 bytes allocated
==9908== 
==9908== All heap blocks were freed -- no leaks are possible
==9908== 
==9908== For counts of detected and suppressed errors, rerun with: -v
==9908== ERROR SUMMARY: 18 errors from 9 contexts (suppressed: 0 from 0)

If you want Data to be copyable, you have to provide a deep copy constructor that allocates new storage and copies data into it. The one in your question looks almost (*) fine:

==9993== HEAP SUMMARY:
==9993==     in use at exit: 0 bytes in 0 blocks
==9993==   total heap usage: 4 allocs, 4 frees, 73,738 bytes allocated
==9993== 
==9993== All heap blocks were freed -- no leaks are possible
==9993== 
==9993== For counts of detected and suppressed errors, rerun with: -v
==9993== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

If you don't need copy construction at all, you can disable its implicit generation by explicitly deleting it:

Data(const Data&) = delete;

The same is true for copy assignment operator. The implicitly-defined one will not do what you want. Don't forget about it.

(*) Note that other.data can be nullptr, so you have to check it before copying:

void* memcpy(void* dest, const void* src, std::size_t count);

If either dest or src is a null pointer, the behavior is undefined, even if count is zero.

Evg
  • 25,259
  • 5
  • 41
  • 83