std::map and _CrtIsValidHeapPointer error

Question

i try to use the std::map with

class DEMO {
    public:
        DEMO();
        virtual ~DEMO();
        DEMO &operator =(const DEMO &d);
        DEMO(const DEMO& d);
        BYTE* Arr() const;
    private:
        BYTE *m_array;
};

DEMO &DEMO::operator =(const DEMO &d) {
    memcpy(m_array, d.Arr(), 1);
    return *this;
}

DEMO::DEMO(const DEMO& d) {
    //call operator=
    *this = d;
}

const BYTE* DEMO::Arr() const {
    return m_array;
}

DEMO::DEMO() {
    m_array = new BYTE[1];
    }

DEMO::~DEMO() {
    if (m_array != 0)
        delete [] m_array;
}

class MyClass {
    private:
        typedef map<unsigned int,DEMO> t_mapType;
        t_mapType m_map;
        void Test();
}

void MyClass::Test() {
    DEMO myDEMO;
    m_map[1] = myDEMO;
}

if i call Test() within the class, i get the error assert _CrtIsValidHeapPointer...
i checked with breakpoints and i see, during assingment (m_map[1] = myDEMO;) the destructor of DEMO gets called and i get the error on delete [] m_array; - how i make this running ?

Just an aside - uppercase names are - by convention - reserved for preprocessor defines. Use them yourself and you can't be sure of portability, and the errors when macro substitutions are accidentally applied can sometimes be subtle and hard to identify. — Tony Delroy, Aug 25 '10 at 08:41

score 4 · Answer 1 · answered Aug 25 '10 at 08:27

4

The copy constructor and assignment operators are wrong. You need to allocate memory for m_array before doing memcpy. When you try to insert a DEMO object into the map using m_map[1] = myDEMO; a copy of the myDEMO is being created. To create a copy the DEMO class's copy ctor is used. Since the memory for m_array is not allocated while doing this copy, the memcpy call is crashing.

answered Aug 25 '10 at 08:27

Naveen

74,600
47
176
233

hm but before the copy ctor, the ctor() is called - this ctor is allocating memory - it crashes by delete[] – gert Aug 25 '10 at 09:13
1

that is allocating memory for a *different* object. – Naveen Aug 25 '10 at 09:23

score 1 · Answer 2 · edited May 23 '17 at 12:26

It's almost always wrong to base the copy ctor implementation on the assignment operator's. An assignment operator has to tear down the object's old state and build a new state. The copy ctor only has to build a new state.
If you want to do it this way, your copy ctor first needs to build a default state for the assignment operator to tear down. Of course, that's wasting resources.

That said, actually you should use the copy-and-swap idiom to base your assignment operator on your dtor and copy ctor instead. See here for a more thorough explanation of the above.

score 0 · Answer 3 · answered Aug 25 '10 at 10:25

Use a std::vector to manage your BYTE*.

class DEMO {
    public:
        DEMO();
        virtual ~DEMO();
        //DEMO &operator =(const DEMO &d);
        //DEMO(const DEMO& d);
        const std::vector<BYTE>& Arr() const;
    private:
        std::vector<BYTE> m_array;
};

// Doesn't even need to be implemented by us anymore
//DEMO &DEMO::operator =(const DEMO &d) {
//    memcpy(m_array, d.Arr(), 1);
//    return *this;
//}

// Again, doesn't need to be implemented
//DEMO::DEMO(const DEMO& d) {
//    //call operator=
//    *this = d;
//}

// Just changed to return const vector&.
const std::vector<BYTE>& DEMO::Arr() const {
    return m_array;
}

// Changed to use initializer list
DEMO::DEMO() 
    : m_array(1) {
    // Old code: m_array = new BYTE[1];
}

// Doesn't need any code by us
DEMO::~DEMO() {
    //if (m_array != 0)
    //    delete [] m_array;
}

score 0 · Answer 4 · answered Aug 25 '10 at 10:43

i changed my code to use pointer to object instead of object for the value part in the map:

typedef map<unsigned int,DEMO*> t_mapType;

so i can assing as follow:

m_map[1] = new DEMO();

and i have to use a own free-mem method:

void DEMO::Clear() {
    if (m_map.size() > 0) {
        for (t_mapType::const_iterator it = m_map.begin(); it != m_mp.end(); ++it) {
            if (it->second != 0)
                delete it->second;
        }
        m_map.clear();
    }
}

You can also use `boost::shared_ptr` to handle the deletion automatically for you. — Naveen, Aug 25 '10 at 10:52

std::map and _CrtIsValidHeapPointer error

4 Answers4