1
\$\begingroup\$

I ran into an access violation reading error and when I debugged it, I saw that the insert function of std::map is inserting a NULL pointer into the map.

Though I've fixed the error by first inserting a NULL value into the map and then changing that value to the corresponding object I wanted to insert in the map. But I want to know the reason the error occurs and why it got fixed. My code looks like this:

void CTextureBank::AddTexture(int ID)
{
    // Create a CTexture object, load texture into it and insert it into the Texture List
    CTexture* texture = new CTexture();
    texture->LoadTextureFromFile(mRenderer, ID);
    mTextureList.insert(std::make_pair<int, CTexture*>(ID, texture));
}

In the AddTexture function, I'm first creating a texture object, loading a texture into it and putting that texture into the map. But, this one gives me an access violation reading error.

And doing this doesn't:

void CTextureBank::AddTexture(int ID)
{
    // Create a CTexture object, load texture into it and insert it into the Texture List
    CTexture* texture = new CTexture();
    texture->LoadTextureFromFile(mRenderer, ID);
    mTextureList.insert(std::make_pair<int, CTexture*>(ID, NULL));
    mTextureList[ID] = texture;
}

So it would be great if someone can tell me why this occurs. Thanks :)

\$\endgroup\$
2
\$\begingroup\$

According to the standard std::map.insert will NOT overwrite existing elements.

Because element keys in a map are unique, the insertion operation checks whether each inserted element has a key equivalent to the one of an element already in the container, and if so, the element is not inserted, returning an iterator to this existing element (if the function returns a value).

So if you already have a mTextureList[ID] that is NULL in your map mTextureList.insert(std::make_pair<int, CTexture*>(ID, texture)); will do absolutely nothing and leave the NULL in place.

Try adding an assert:

void CTextureBank::AddTexture(int ID)
{
    // Create a CTexture object, load texture into it and insert it into the Texture List
    assert(mTextureList.find(ID) == mTextureList.end()); // Check if ID already in map

    CTexture* texture = new CTexture();
    texture->LoadTextureFromFile(mRenderer, ID);
    mTextureList.insert(std::make_pair<int, CTexture*>(ID, texture));
}

This will tell you if there's already a NULL in the map.

The easy way to always insert and overwrite is to just use mTextureList[ID] = texture; and not use std::map.insert but you will possibly leak memory if you don't fist delete the previous CTexture.

For basic types there is no significant impact to using the [] operator to insert/overwrite into a map. The value in the map's pair gets initialized to zero/NULL then overwritten in the assignment.

In fact the cost of passing std::make_pair to .insert may be larger but this depends on compilers and compiler settings.

void CTextureBank::AddTexture(int ID)
{
    // Create a CTexture object, load texture into it and insert it into the Texture List
    CTexture* texture = new CTexture();
    texture->LoadTextureFromFile(mRenderer, ID);

    auto i = mTextureList.find(ID);
    // If you don't have C++11 enabled instead of auto use:
    // std::map<int, CTexture*>::iterator i = mTextureList.find(ID);

    if(i == mTextureList.end()){
        // entry does not exist, insert new one
        mTextureList.insert(std::make_pair<int, CTexture*>(ID, texture));
    } else {
      // There's already an entry of this ID
      if(i->second){
          MESSAGE("WOOPS!, THERE'S ALREADY A TEXTURE WITH THIS ID. IS THIS OKAY?\n");
          delete i->second; // delete old texture ??? Is this safe to do?
      } else {
          MESSAGE("It's just a null, we're fine.\n");
          // Someone probably used mTextureList[ID] which created a NULL
      }
      i->second = texture; // overwrite existing entry's value
    }
}

Or shorter:

void CTextureBank::AddTexture(int ID)
{
    // Create a CTexture object, load texture into it and insert it into the Texture List
    CTexture* texture = new CTexture();
    texture->LoadTextureFromFile(mRenderer, ID);
    auto result = mTextureList.insert(std::make_pair<int, CTexture*>(ID, texture));
    if(!result.second){
       if(result.first->second != NULL){
           MESSAGE("WOOPS!, THERE'S ALREADY A TEXTURE WITH THIS ID. IS THIS OKAY?\n");
           delete result.first->second; // delete old texture
       }
       result.first->second = texture; // save new texture
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ So if you already have a mTextureList[ID]that is NULL in your map mTextureList.insert(std::make_pair<int, CTexture*>(ID, texture)); will do absolutely nothing and leave the NULL in place. But it does, it overwrites the NULL and puts a texture into it. So how can this be true? \$\endgroup\$
    – Se7eN
    Dec 24 '17 at 15:02
  • 1
    \$\begingroup\$ @Ashar7 Read the page I linked with the quote cplusplus.com/reference/map/map/insert , there's an example at the bottom. insert does NOT overwrite the existing values. The documentation is quite clear about that. \$\endgroup\$ Dec 24 '17 at 15:29
  • \$\begingroup\$ Okay, I got it. \$\endgroup\$
    – Se7eN
    Dec 25 '17 at 15:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.