std::map inserts a NULL pointer

Question

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 :)

Answer 1

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
    }
}