Search code examples
c++csdl

Behaviour of function SDL_FreeSurface: why it crashes in this context?


I am learning SDL with an online tutorial and found a problem I do not understand. Accordingly to the tutorial, it is good practice to always correctly free the resources used by your application. In my case, I want to free the memory used by the SDL surfaces.

However, one SDL_FreeSurface crashes my program in the exit if I do not set the pointer it is going to free to nullptr BEFORE calling the function.

This does not occur with any other surfaces. Even solving the problem by changing order of statements, I dislike ''cargo cult''. I want to understand WHY in this specific case I need to make the pointer nullptr BEFORE freeing the memory.

This is the code:

#define SDL_MAIN_HANDLED
#include<SDL2/SDL.h>

int main (void) {SDL_SetMainReady();

    SDL_Init(SDL_INIT_VIDEO);

    enum image_list{ /* set enum */ };

    SDL_Window  * ptr_Window = nullptr;
    SDL_Surface * ptr_Canvas = nullptr;
    SDL_Surface * ptr_Mask   = nullptr;     // this is the pointer with problem
    SDL_Surface * array_list_of_images[enum_Total]; /* each array initialized correctly */

    ptr_Mask = array_list_of_images[enum_Image_Default]; // here the problematic pointer is set to array[0]

    /* code */

    // Here is where the pointer is used:
    switch (union_events_holder.key.keysym.sym){
        case SDLK_UP    : ptr_Mask = array_list_of_images[enum_Image_Up]; break;
        case SDLK_DOWN  : ptr_Mask = array_list_of_images[enum_Image_Down]; break;
        case SDLK_LEFT  : ptr_Mask = array_list_of_images[enum_Image_Left]; break;
        case SDLK_RIGHT : ptr_Mask = array_list_of_images[enum_Image_Right]; break;
        default         : ptr_Mask = array_list_of_images[enum_Image_Default]; break;

    /* more code */

    SDL_BlitSurface(ptr_Mask , NULL , ptr_Canvas , NULL );
    SDL_UpdateWindowSurface(ptr_Window);

    /* more code */

    // HERE OCCURS THE PROBLEM.
    // WITHOUT POINTING ptr_Mask TO NULL BEFORE CALLING SDL_FreeSurface,
    // THE APP CRASHES.

    ptr_Mask    = nullptr;
    SDL_FreeSurface(ptr_Mask); // This cannot be called before previous statement. Why?

    for ( int ctd = 0 ; ctd < enum_Total ; ++ctd){
        SDL_FreeSurface(array_list_of_images[ctd]);
        array_list_of_images[ctd] = nullptr; // Here it is safe to set pointer to nullptr AFTER freeing memory.
    }   

    SDL_FreeSurface(ptr_Canvas);   
    SDL_DestroyWindow(ptr_Window);

    ptr_Canvas       = nullptr; // Here also it is safe to set pointer to nullptr AFTER freeing memory.
    ptr_Window       = nullptr;

    SDL_Quit();

return (0);}

Notes:

  • Tagged as C because SDL. I am coding with C++.

  • Found the bug proceeding manual step-by-step with std::cerr + std::cin.get()

  • SDL_GetError() cannot output info because both Window and console crash when the function is called.

  • I suspected of undefined behaviour of dereferencing nullptr, but it makes no sense, once the function only works exactly when I set to nullptr.

EDIT - This is the full code (variables in Brazilian portuguese)

#define SDL_MAIN_HANDLED
#include<SDL2/SDL.h>

int main (void) {SDL_SetMainReady();

    SDL_Init(SDL_INIT_VIDEO);

    SDL_Window  * ptr_Janela = nullptr;
    SDL_Surface * ptr_Tela   = nullptr;

    enum lista_de_imagens{
        LIMG_Imagem_Default, 
        LIMG_Imagem_Cima,
        LIMG_Imagem_Baixo,
        LIMG_Imagem_Esquerda,
        LIMG_Imagem_Direita,
        LIMG_Total_de_imagens
    };

    SDL_Surface * l_lista_de_imagens[LIMG_Total_de_imagens]; 

        l_lista_de_imagens[LIMG_Imagem_Default]   = SDL_LoadBMP("Lesson4/Default.bmp" );
        l_lista_de_imagens[LIMG_Imagem_Cima]      = SDL_LoadBMP("Lesson4/Cima.bmp"    );
        l_lista_de_imagens[LIMG_Imagem_Baixo]     = SDL_LoadBMP("Lesson4/Baixo.bmp"   );
        l_lista_de_imagens[LIMG_Imagem_Esquerda]  = SDL_LoadBMP("Lesson4/Esquerda.bmp");
        l_lista_de_imagens[LIMG_Imagem_Direita]   = SDL_LoadBMP("Lesson4/Direita.bmp" );
        l_lista_de_imagens[LIMG_Total_de_imagens ]= nullptr;

    SDL_Surface * ptr_mascara = nullptr;

    ptr_Janela = SDL_CreateWindow("Lesson 4 - Show image as keyboard inputs",
                                SDL_WINDOWPOS_UNDEFINED,
                                SDL_WINDOWPOS_UNDEFINED,
                                640, //SCREEN_WIDTH,
                                480, //SCREEN_HEIGHT,
                                SDL_WINDOW_SHOWN);
    ptr_Tela = SDL_GetWindowSurface(ptr_Janela);

    SDL_BlitSurface(l_lista_de_imagens[LIMG_Imagem_Default] , NULL , ptr_Tela , NULL );
    SDL_UpdateWindowSurface(ptr_Janela);

    SDL_Event u_Gerenciador_de_eventos;
    bool sair = false;

    ptr_mascara = l_lista_de_imagens[LIMG_Imagem_Default];

    while(!sair){

            while(SDL_PollEvent(&u_Gerenciador_de_eventos) !=0){

                if(u_Gerenciador_de_eventos.type == SDL_QUIT){sair = true;}
                if( u_Gerenciador_de_eventos.type == SDL_KEYDOWN){

                    switch (u_Gerenciador_de_eventos.key.keysym.sym){

                        case SDLK_UP    : ptr_mascara = l_lista_de_imagens[LIMG_Imagem_Cima]; break;

                        case SDLK_DOWN  : ptr_mascara = l_lista_de_imagens[LIMG_Imagem_Baixo]; break;

                        case SDLK_LEFT  : ptr_mascara = l_lista_de_imagens[LIMG_Imagem_Esquerda]; break;

                        case SDLK_RIGHT : ptr_mascara = l_lista_de_imagens[LIMG_Imagem_Direita]; break;

                        default         : ptr_mascara = l_lista_de_imagens[LIMG_Imagem_Default]; break;

                    /*end switch*/}

                /*end if*/}

        /*end laço de eventos*/}

    SDL_BlitSurface(ptr_mascara , NULL , ptr_Tela , NULL );
    SDL_UpdateWindowSurface(ptr_Janela);

    /*end laço principal*/}

    ptr_mascara    = nullptr;
    SDL_FreeSurface(ptr_mascara); 

    for ( int ctd = 0 ; ctd < LIMG_Total_de_imagens ; ++ctd){
        SDL_FreeSurface(l_lista_de_imagens[ctd]);
        l_lista_de_imagens[ctd] = nullptr;
    /*end for*/}    

    SDL_FreeSurface(ptr_Tela);   
    SDL_DestroyWindow(ptr_Janela);

    ptr_Tela       = nullptr;
    ptr_Janela     = nullptr;

    SDL_Quit();

return (0);
/*end main*/}

Solution

  • The selected surface gets freed twice.

    You have two pointers to the same image. The first one is in l_lista_de_imagens[LIMG_Imagem_Default] and the second one is in ptr_mascara.

    When you do SDL_FreeSurface(ptr_mascara) and then SDL_FreeSurface(l_lista_de_imagens[ctd]); you are freeing the same surface twice.

    SDL_FreeSurface(nullptr) doesn't do anything, so when you add ptr_mascara = nullptr; before SDL_FreeSurface(ptr_mascara), you are only freeing the surface once.