Recently I've been trying to learn SDL, a graphics library amongst other things, for C. I haven't gotten very far but with the basics I've learnt from this tutorial (Yes, I know it's for C++ but it seems the majority of things are still the same), I've tried to create a "template" SDL program to start all my other future programs from. This is what I had:
#include <stdio.h>
#include <stdlib.h>
#include <SDL.h>
int initProg(SDL_Window **window, SDL_Surface **surface, char *name, int x, int y, int w, int h, int flags);
void mainLoop(SDL_Window *window, SDL_Surface *surface);
void closeProg(SDL_Window *window);
SDL_Surface *loadImage(char *path);
int main(int argc, char *args[]) {
SDL_Window *window = NULL;
SDL_Surface *surface = NULL;
char windowName[13] = "SDL Tutorial";
int windowXPos = SDL_WINDOWPOS_UNDEFINED;
int windowYPos = SDL_WINDOWPOS_UNDEFINED;
int windowWidth = 600;
int windowHeight = 600;
int flags = SDL_WINDOW_SHOWN;
if (!initProg(&window, &surface, windowName, windowXPos, windowYPos, windowWidth, windowHeight, flags)) {
return 1;
}
mainLoop(window, surface);
closeProg(window);
return 0;
}
int initProg(SDL_Window **window, SDL_Surface **surface, char *name, int x, int y, int w, int h, int flags) {
if (SDL_Init(SDL_INIT_EVERYTHING) < 0) {
printf("Failed to initialize SDL.\nError: %s\n", SDL_GetError());
return 0;
} else {
*window = SDL_CreateWindow(name, x, y, w, h, flags);
if (*window == NULL) {
printf("Failed to create a window.\nError:%s\n", SDL_GetError());
return 0;
} else {
*surface = SDL_GetWindowSurface(*window);
return 1;
}
}
}
void mainLoop(SDL_Window *window, SDL_Surface *surface) {
// Simple program to fade between white and black background
int g = 0;
int diff = -1;
int quit = 0;
SDL_Event e;
while (!quit) {
while(SDL_PollEvent(&e) != 0) {
if (e.type == SDL_QUIT) {
quit = 1;
}
}
SDL_FillRect(surface, NULL, SDL_MapRGB(surface->format, g, g, g));
SDL_UpdateWindowSurface(window);
if (g == 0 || g == 255) {
diff *= -1;
}
g += diff;
SDL_Delay(10);
}
}
void closeProg(SDL_Window *window) {
SDL_DestroyWindow(window);
window = NULL;
SDL_Quit();
}
The program is divided into three sections: one to initialize SDL, one where the main program occurs, and one to close SDL and free the surfaces.
I realized the problem with this is that if I had created another surface then I would have to add code to the closeProg() function to free the surface at the end. N.B: Keep in mind that these "surfaces" are just pointers to the actual thing, which as far as I'm aware you don't really interact with, you just deal with the pointer to it.
To work around this, I decided to create an array of these pointers-to-surfaces, and then make a function that creates a surface and adds it's pointer to the array. This would allow me at the end in the closeProg() function to go through each surface and free it, then free the array too. (Note that the window's surface is not added to this list as it gets freed automatically with the SDL_DestroyWindow() function)
Here is the declaration of this new array:
// The pointer to the array. Currently NULL until something is added
SDL_Surface **surfaces = NULL;
// Keeps track of the size of the array
size_t surfaceCount = 0;
This is the function that adds to the array:
int addSurface(SDL_Surface *surface, SDL_Surface **surfaces, size_t *surfaceCount) {
size_t new_count = *surfaceCount + 1;
SDL_Surface **temp = realloc(surfaces, sizeof(*temp) * new_count);
if (temp == NULL) {
printf("Failed to reallocate to %d bytes of memory.", sizeof(*temp) * new_count);
return 0;
} else {
surfaces = temp;
*(surfaces + new_count - 1) = surface;
*surfaceCount = new_count;
return 1;
}
}
And here it is in use. Note that loadImage() returns a surface from an image, and showSurfaces() prints out the contents of the surfaces array.
// Check contents before adding anything
showSurfaces(surfaces, *surfaceCount);
// Add an image
SDL_Surface *fire = loadImage("./fire.bmp");
addSurface(fire, surfaces, surfaceCount);
// Check contents again
showSurfaces(surfaces, *surfaceCount);
// Add another image and check contents again
SDL_Surface *ice = loadImage("./ice.bmp");
addSurface(ice, surfaces, surfaceCount);
showSurfaces(surfaces, *surfaceCount);
// Add another image and check contents a final time
SDL_Surface *man = loadImage("./man.bmp");
addSurface(man, surfaces, surfaceCount);
showSurfaces(surfaces, *surfaceCount);
Up to the first two surfaces, everything went well. However, when I tried to add the third surface, the program hanged immediately before closing (indication that something is wrong in the closeProg() function?). I decided to print out the contents of the array and here is what I got.
No current surfaces.
Current surfaces:
Index 0: 00753C98
Current surfaces:
Index 0: 00753C98
Index 1: 00754780
Current surfaces:
Index 0: 02805150
Index 1: 008F00C0
Index 2: 201339FC
In the first two print-outs, everything seemed well but in the third one, you can notice that the previous addresses have changed. This happened repeatedly and no matter how many surfaces I added. After the first two reallocations, the array's content kept on changing.
I think that is why the program hangs when closing, because in the closeProg function, the program is told to free an unknown pointer that is not a surface, hence it crashes. Not to mention that I also set that pointer to NULL, and who knows what else that could cause.
Is this changing of content of the array normal? And if not, I would really appreciate if you could help me find what is causing this strange behavior. I repeat that I am a COMPLETE BEGINNER in this and so any help, even in a matter not relating to this question would be GREATLY appreciated. Thank you in advance :)
For reference, here are the images I used.
Here is the full code if you're interested:
#include <stdio.h>
#include <stdlib.h>
#include <SDL.h>
int initProg(SDL_Window **window, SDL_Surface **surface, char *name, int x, int y, int w, int h, int flags);
void mainLoop(SDL_Window *window, SDL_Surface *surface, SDL_Surface **surfaces, size_t *surfaceCount);
void closeProg(SDL_Window *window, SDL_Window **surfaces, size_t surfaceCount);
SDL_Surface *loadImage(char *path);
int addSurface(SDL_Surface *surface, SDL_Surface **surfaces, size_t *surfaceCount);
int main(int argc, char *args[]) {
SDL_Window *window = NULL;
SDL_Surface *surface = NULL;
// The pointer to the array. Currently NULL untill something is added
SDL_Surface **surfaces = (SDL_Surface *)calloc(1, sizeof(*surfaces));
// Keeps track of the size of the array
size_t surfaceCount = 0;
char windowName[13] = "SDL Tutorial";
int windowXPos = SDL_WINDOWPOS_UNDEFINED;
int windowYPos = SDL_WINDOWPOS_UNDEFINED;
int windowWidth = 600;
int windowHeight = 600;
int flags = SDL_WINDOW_SHOWN;
if (!initProg(&window, &surface, windowName, windowXPos, windowYPos, windowWidth, windowHeight, flags)) {
return 1;
}
mainLoop(window, surface, surfaces, &surfaceCount);
closeProg(window, surfaces, surfaceCount);
return 0;
}
int initProg(SDL_Window **window, SDL_Surface **surface, char *name, int x, int y, int w, int h, int flags) {
if (SDL_Init(SDL_INIT_EVERYTHING) < 0) {
printf("Failed to initialize SDL.\nError: %s\n", SDL_GetError());
return 0;
} else {
*window = SDL_CreateWindow(name, x, y, w, h, flags);
if (*window == NULL) {
printf("Failed to create a window.\nError:%s\n", SDL_GetError());
return 0;
} else {
*surface = SDL_GetWindowSurface(*window);
return 1;
}
}
}
void mainLoop(SDL_Window *window, SDL_Surface *surface, SDL_Surface **surfaces, size_t *surfaceCount) {
// Simple program to fade between white and black background
int g = 0;
int diff = -1;
// Check contents before adding anything
showSurfaces(surfaces, *surfaceCount);
// Add an image
SDL_Surface *fire = loadImage("./fire.bmp");
addSurface(fire, surfaces, surfaceCount);
// Check contents again
showSurfaces(surfaces, *surfaceCount);
// Add another image and check contents again
SDL_Surface *ice = loadImage("./ice.bmp");
addSurface(ice, surfaces, surfaceCount);
showSurfaces(surfaces, *surfaceCount);
// Add another image and check contents a final time
SDL_Surface *man = loadImage("./man.bmp");
addSurface(man, surfaces, surfaceCount);
showSurfaces(surfaces, *surfaceCount);
int quit = 0;
SDL_Event e;
while (!quit) {
while(SDL_PollEvent(&e) != 0) {
if (e.type == SDL_QUIT) {
quit = 1;
}
}
SDL_FillRect(surface, NULL, SDL_MapRGB(surface->format, g, g, g));
SDL_UpdateWindowSurface(window);
if (g == 0 || g == 255) {
diff *= -1;
}
g += diff;
SDL_Delay(10);
}
}
void closeProg(SDL_Window *window, SDL_Window **surfaces, size_t surfaceCount) {
// Go through the array and free each surface.
for (int i = 0; i < surfaceCount; i++){
SDL_FreeSurface(*(surfaces + i));
*(surfaces + i) = NULL;
}
// Free the array itself.
free(surfaces);
SDL_DestroyWindow(window);
window = NULL;
SDL_Quit();
}
SDL_Surface *loadImage(char *path) {
SDL_Surface *image = SDL_LoadBMP(path);
if (image == NULL) {
printf("Failed to load image.\nError: %s\n", SDL_GetError());
}
return image;
}
int addSurface(SDL_Surface *surface, SDL_Surface **surfaces, size_t *surfaceCount) {
size_t new_count = *surfaceCount + 1;
SDL_Surface **temp = realloc(surfaces, sizeof(*temp) * new_count);
if (temp == NULL) {
printf("Failed to reallocate to %d bytes of memory.", sizeof(*temp) * new_count);
return 0;
} else {
surfaces = temp;
*(surfaces + new_count - 1) = surface;
*surfaceCount = new_count;
return 1;
}
}
void showSurfaces(SDL_Surface **surfaces, size_t surfaceCount) {
if (surfaceCount == 0) {
printf("\nNo current surfaces.\n");
} else {
printf("\nCurrent surfaces:\n");
for (int i = 0; i < surfaceCount; i++) {
printf("\tIndex %d: %p\n", i, *(surfaces + i));
}
putchar('\n');
}
}
If you can replicate this error, please comment down below so I know it's not something wrong with my machine or anything like that.
NOTE: I am using SDL 2.0
I realized the problem with this is that if I had created another surface then I would have to add code to the closeProg() function to free the surface at the end.
Not really. It's cleanest to do that, but the OS can be relied upon to release the program's memory, including any remaining dynamically allocated memory, when the program terminates. You could as easily just hold the allocated memory until then, instead of freeing it explicitly just prior to termination.
Anyway,
In the first two print-outs, everything seemed well but in the third one, you can notice that the previous addresses have changed. This happened repeatedly and no matter how many surfaces I added. After the first two reallocations, the array's content kept on changing.
I think that is why the program hangs when closing, because in the closeProg function, the program is told to free an unknown pointer that is not a surface, hence it crashes. Not to mention that I also set that pointer to NULL, and who knows what else that could cause.
I think your analysis is plausible.
Is this changing of content of the array normal?
No. Reallocation with realloc() preserves the contents of the original space, copying it to the new space if it does not overlap, up to the lesser of the sizes of the two spaces.
I would really appreciate if you could help me find what is causing this strange behavior.
Your addSurface() function is flawed. Although you have done well with the details of the reallocation itself, accounting both for the possibility that reallocation does not occur in place and the possibility that it fails, you do not successfully convey the new pointer to the caller.
In particular, consider this excerpt:
surfaces = temp;
*(surfaces + new_count - 1) = surface;
*surfaceCount = new_count;
The surface and surfaceCount variables are both function parameters. You seem to understand that in order to convey a new surface count value back to the caller via the latter, it must be a pointer to a variable accessible to the caller, so that the function can update that value via the pointer. That's what the last line of the excerpt does.
The situation is no different for the surfaces pointer. When in the first line of the excerpt you assign to surfaces, you are not making a change that will be visible to the caller. You're only modifying the value of a local variable inside the function. You must either add a layer of indirection for surface, or else convey the new pointer back to the caller via the function's return value. Personally, I'd go with the latter, because three-* programming is not widely accepted as good style.