C++ possible constructor error violation writing location

Question

I have added these functions to my code, and my program started crashing randomly showing Unhandled exception at 0x6C7B52E7 (SDL2.dll) in SDL_2.exe: 0xC0000005: Access violation writing location 0xFF00000C. Debugger shows at completely random pieces of code, that worked perfectly before, which leads me to think, that these new functions mess up the memory.

void ObstacleSet::move()
{
Obstacles.push_back(Obstacle(mScreenWidth, mScreenHeight / 6 + 100, 10, 10, Coin, -29, 0));
Obstacles.push_back(Obstacle(mScreenWidth, mScreenHeight / 6 + 120, 10, 10, Coin, -29, 0));
Obstacles.push_back(Obstacle(mScreenWidth, mScreenHeight / 6 + 140, 10, 10, Coin, -29, 0));
Obstacles.push_back(Obstacle(mScreenWidth, mScreenHeight / 6 + 160, 10, 10, Coin, -29, 0));
Obstacles.push_back(Obstacle(mScreenWidth, mScreenHeight / 6 + 180, 10, 10, Coin, -29, 0));

for (auto &i : Obstacles)
    {
        i.move();
    }
}

void ObstacleSet::outCheck(unsigned int &score)
{
    for (unsigned int i = 0; i < getSize();)
    {
        if (getVelX(i) < 0 && (getX(i) + getW(i) <= 0))
        {
            eraseObstacle(i);
            score++;
        }

        else if (getVelX(i) > 0 && (getX(i) >= mScreenWidth))
        {
            eraseObstacle(i);
            score++;
        }

        else if (getVelY(i) > 0 && (getY(i) + getH(i) >= mScreenHeight))
        {
            eraseObstacle(i);
            score++;
        }

        else if (getVelY(i) < 0 && (getY(i) <= 0))
        {
            eraseObstacle(i);
            score++;
        }

        else
        {
            ++i;
        }
    }
}

Apparently, i have some memory leak, or a dangling pointer somehow appears, but i just can not see where. Maybe i have a wrong constructor or something.

ObstacleSet is a class, that holds all Obstacles in a list.

#ifndef MOBSTSET_H
#define MOBSTSET_H

#include <SDL.h>
#include <cstdlib>
#include "my_OBSTACLES.h"
#include <list>

class ObstacleSet
{
public:

ObstacleSet(int ScreenWidth, int ScreenHeight);

int mScreenWidth;
int mScreenHeight;

void clear();

//Function, that moves the set depending on current time
void move();

//Render set on screen
void render(SDL_Renderer *Renderer);

unsigned int getSize();

float getX(int);
float getY(int);
int getW(int);
int getH(int);
float getVelX(int);
float getVelY(int);
Type GetType(int);

void eraseObstacle(int);

void outCheck(unsigned int &score);

std::list<Obstacle>::iterator getEnd();

private:

//Vector to hold all obstacles
std::list <Obstacle> Obstacles;

};
#endif

These appear in my main game loop one after another

Obstacle class header

#ifndef MYOBSTACLE_H
#define MYOBSTACLE_H

#include <SDL.h>
#include <cmath>
#include <vector>
#include <cstdlib>

enum Type
{
Wall,
Coin
};

class Obstacle
{
public:
Obstacle();

//Initializer with parameters
Obstacle(float x, float y, int w, int h, Type type, float VelocityX = 0, float VelocityY = 0);

float VelocityX = 0;
float VelocityY = 0;

float xPos;
float yPos;

int mWidth;
int mHeight;

//Type of obstacle
Type OBSType;

//Render obstacle on screen
void render(SDL_Renderer *Renderer);

//Move obstacle
void move();

int mRed = 200;
int mGreen = 200;
int mBlue = 20;
bool color_up = false;
};

#endif

These are my constructors for Obstacle

Obstacle::Obstacle() : xPos(10), yPos(10), mWidth(10), mHeight(10), VelocityX(0), VelocityY(0), OBSType(Coin), mRed(200), mGreen(200), mBlue(20), color_up(false)
{
    printf("Obstacle created!");
}

Obstacle::Obstacle(float x, float y, int w, int h, Type type, float velocityX, float velocityY) : xPos(x), yPos(y), mWidth(w), mHeight(h), VelocityX(velocityX), VelocityY(velocityY), OBSType(type), mRed(200), mGreen(200), mBlue(20), color_up(false)
{

}

Here are some functions of ObstacleSet

void ObstacleSet::eraseObstacle(int number)
{
    unsigned N = number;
    std::list<Obstacle>::iterator i = Obstacles.begin();
    if (Obstacles.size() > N)
    {
        std::advance(i, N);
    }
        Obstacles.erase(i);
}

unsigned int ObstacleSet::getSize()
{
    return Obstacles.size();
}

float ObstacleSet::getX(int number)
{
    unsigned N = number;
    std::list<Obstacle>::iterator i = Obstacles.begin();
    if (Obstacles.size() > N)
    {
        std::advance(i, N);
    }
    return i->xPos;
}

std::list<Obstacle>::iterator ObstacleSet::getEnd()
{
    return Obstacles.end();
}

SDL Initialization code

bool init()
{
    //Initialization flag
    bool success = true;

    //Initialize SDL
    if (SDL_Init(SDL_INIT_VIDEO | SDL_INIT_JOYSTICK | SDL_INIT_AUDIO) < 0)
    {
        printf("SDL could not initialize! SDL Error: %s\n", SDL_GetError());
        success = false;
    }
    else
        //Set texture filtering to linear
        if (!SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "1"))
        {
            printf("Warning: Linear texture filtering not enabled!");
        }

    //Initialize SDL_mixer
    if (Mix_OpenAudio(44100, MIX_DEFAULT_FORMAT, 2, 2048) < 0)
    {
        printf("SDL_mixer could not initialize! SDL_mixer Error: %s\n", Mix_GetError());
        success = false;
    }

    //Check for joysticks
    if (SDL_NumJoysticks() < 1)
    {
        printf("Warning: No joysticks connected!\n");
    }
    else
    {
        //Load joystick
        gGameController = SDL_JoystickOpen(0);
        if (gGameController == NULL)
        {
            printf("Warning: Unable to open game controller! SDL Error: %s\n", SDL_GetError());
        }
    }

    //Create window
    gWindow = SDL_CreateWindow("Jello demo", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN);
    if (gWindow == NULL)
    {
        printf("Window could not be created! SDL Error: %s\n", SDL_GetError());
        success = false;
    }
    else
    {
        //Create vsynced renderer for window
        gRenderer = SDL_CreateRenderer(gWindow, -1, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);
        if (gRenderer == NULL)
        {
            printf("Renderer could not be created! SDL Error: %s\n", SDL_GetError());
            success = false;
        }
        else
            //Initialize renderer color
            SDL_SetRenderDrawColor(gRenderer, 0xFF, 0xFF, 0xFF, 0xFF);

        //Initialize PNG loading
        int imgFlags = IMG_INIT_PNG;
        if (!(IMG_Init(imgFlags) & imgFlags))
        {
            printf("SDL_image could not initialize! SDL_image Error: %s\n", IMG_GetError());
            success = false;
        }

        //Initialize SDL_ttf
        if (TTF_Init() == -1)
        {
            printf("SSDL_ttf could not initialize! SDL_ttf Error: %s\n", TTF_GetError());
            success = false;
        }
    }
    return success;
}

Maybe I have got a memory leak or a dangling pointer, but I can't find where the mistake is. I will be VERY glad if anyone could find where i am wrong. Write me if you need any additional code pieces

Have you tried using the debugger and stepping through the code to see which line the program breaks at? — Zara Kay, Apr 22 '15 at 22:14
@MigaraLiyanagamage Debugger shows at completely random pieces of code, that worked perfectly before, which leads me to think, that new functions mess up the memory. — gavriktonio, Apr 22 '15 at 22:34
do you have subclasses of Obstacle and add them to your obstacles list? — Diversity, Apr 22 '15 at 22:39
It seems that you even don't have any dynamic allocated pointers or objects is that correct? When you say you have added some function and after that it didn't work can you emphasize the added functions and the state before — Diversity, Apr 22 '15 at 22:45
@Diversity As far as i understand, the only dynamicly allocated thing in my code is Obstacles list. I have added three functions - move, OutCheck, and eraseObstacle. If i comment them, everything works just fine. My guess is that memory is not managed properly when new objects are created or deleted. — gavriktonio, Apr 22 '15 at 22:52
a dynamic allocated object is declared using a star "*" e.G. within the header file Obstacle *obstacle. To allocate memory for this object you have to initialize it by calling the new operator. Obstacle *o = new Obstacle() which will call the default constructor of your class Obstacle. So i can't see any new operators within your given code. That means that a dangling pointer or memory leak is currently not visible for me because all Memory allocations and deallocations are made implicetely. — Diversity, Apr 22 '15 at 22:57
That's a large amount of code, please try to produce a [MCVE](http://stackoverflow.com/help/mcve) — M.M, Apr 22 '15 at 22:59

score 0 · Answer 1 · answered Apr 22 '15 at 22:57

Unless you have an invariant somewhere that guarantees that ObstacleSet never calls anything on an empty Obstacles list there is a potential problem.

This code has undefined behaviour on an empty list

float ObstacleSet::getX(int number)
{
    unsigned N = number;
    std::list<Obstacle>::iterator i = Obstacles.begin();
    if (Obstacles.size() > N)
    {
        std::advance(i, N);
    }
    return i->xPos;
}

Trying to fix it

float ObstacleSet::getX(int number)
{
    unsigned N = number;
    std::list<Obstacle>::iterator i = Obstacles.begin();
    if (Obstacles.size() > N)
    {
        std::advance(i, N);
    }
    if (i != Obstacles.end())
        return i->xPos;
    // handle empty
    return ERRORPOS; // or throw or assert or ...
}

Same with all other functions that access the list.

@Diversity, either I need more sleep or this comment was not intended for me. — Surt, Apr 22 '15 at 23:10

score 0 · Answer 2 · answered Apr 22 '15 at 22:58

Some issues I see in the code:

In ObstacleSet::move() you add five obstacles to the list. It looks like the list can grow indefinitely, resulting in memory overflow.
You don't check if Obstacles are empty in eraseObstacle(), getX() and perhaps in other functions which you haven't shown.

score 0 · Accepted Answer · answered Apr 23 '15 at 18:50

So apparently, there error was not in the code i provided. I initialized SDL_mixer wrongly. I loaded audio like this if (Mix_OpenAudio(44100, MIX_DEFAULT_FORMAT, 2, 2048) < 0), but sampling rate of wav file i provided was not MIX_DEFAULT_FORMAT, which is 22050. Anyway, thanks to everybody, who tried to help me with my code. It was a pleasant experience to use stackoverflow for the fisrt time.

C++ possible constructor error violation writing location

3 Answers3