C++ copy constructor 0xC0000005: Access violation writing error

Question

Here is the code. This is a class that stores points in the R^2 space.

PointSet::PointSet(const PointSet& ps)
{
    if (Containment < ps.Size)
    {
        if (_Set != nullptr) delete[]_Set;
        _Set = new Point[ps.Size];
        Containment = ps.Size;
    }
    Size = ps.Size;
    for (unsigned i = 0; i != ps.Size; ++i)
    {
        _Set[i] = ps._Set[i];
    }
}

A 0xC0000005:Access violation writing error occured when I pass a PointSet object to a function's parameter. I wonder if it is because the parameter stores in the function's stack and there is not efficient space for the PointSet object(stackoverflow?lol)? But I think the keyword new won't allocate space in the stack. So I am confused, why is this error happend? Thanks!

identifiers starting with double underscore `__` or with an underscore and an upper letter `_X` are [reserved](http://stackoverflow.com/q/228783/2805305). So `_Set` is an invalid name. You should refactor your code. — bolov, Dec 02 '15 at 13:33
I'm sorry, what are you trying to achieve exactly? If you are learning how to write your own vector class, why don't you use a generic solution? Why are you using raw pointers instead of smart ones? If this is a class in a real-world project, why not just stick to standard vector class? — RX_DID_RX, Dec 03 '15 at 05:54
I tried to change to use the vector class but still get stack overflowed. — Coldice Ray Esedra, Dec 03 '15 at 06:53
Please ask a separate question for this new issue you are having. Also, take into account that the problem might be in an unrelated area of your code. The process of creating a [minimal complete example of the problem](http://stackoverflow.com/help/mcve) often proves instrumental in resolving it before even asking the question. — user4815162342, Dec 03 '15 at 10:42
@user4815162342 I would like to ask another separate question, but I couldn't because the system said I could ask another question 2 days later. — Coldice Ray Esedra, Dec 03 '15 at 10:47
The code you've shown seems correct. It's hard to tell what's wrong without seeing a complete (and small) example that crashes. For example, you could be forgetting to initialize `Containment`, leading to it being negative, `(++Size > Containment)` being true, and `operator new[]` receiving a small negative (= huge positive) number. Or, you could have a memory corruption elsewhere in your code that breaks `operator new[]`. — user4815162342, Dec 03 '15 at 10:58
@user4815162342 Thank you very much! I have updated the question, it seems is because the recursion I use is too deep, so it ran out of the stack space. I tried to reduce the data size and error disappeared. So I think increase the stack size would do the same. I'll try to do that later. — Coldice Ray Esedra, Dec 03 '15 at 11:19

user4815162342 · Accepted Answer · 2015-12-02T16:33:10.907

4

Don't deallocate (or read) this->_Set in the copy constructor - it will be called on an unconstructed instance in order to construct it. The code you showed would be appropriate for the assignment operator (along with the this != &ps check), which is invoked to change the contents of an already constructed object. A correct copy constructor would look like this:

PointSet::PointSet(const PointSet& ps)
{
    Containment = ps.Size;
    _Set = new Point[Containment];
    Size = ps.Size;
    for (size_t i = 0; i != ps.Size; ++i)
        _Set[i] = ps._Set[i];
}

Or, better yet, replace the raw pointer/size/containment members with a standard container that encapsulates the three, such as std::vector<Point>. Then the job of your class will be to expose the public API that makes sense for the domain you are modeling. On the other hand, the use of std::vector will ensure that the special member functions auto-generated by the compiler (copy constructor, destructor, and assignment operators) are efficient and correct.

edited Dec 02 '15 at 16:33

answered Dec 02 '15 at 13:18

user4815162342

141,790
18
296
355

Oh, I see. Thanks a lot. But what if I want to copy a PointSet instance to another one? Do I have to overwrite the = operator? – Coldice Ray Esedra Dec 02 '15 at 13:23
@ColdiceRayEsedra Yes, when you customize the destructor, you almost certainly want to customize the copy constructor and the assignment operator (look up the "rule of three"). – user4815162342 Dec 02 '15 at 13:25
@ColdiceRayEsedra And furthermore, if you implement `PointSet` using a `std::vector` member, the auto-generated destructor, assignment, and copy constructor will work just fine. (And you will automatically get move constructor and move assignment under C++11.) – user4815162342 Dec 02 '15 at 13:26

C++ copy constructor 0xC0000005: Access violation writing error

1 Answers1