shithub: cstory

Info  •  Files  •  Log  •  Branches

Added sanity checks to the backends

This fixes the Texture backend bug that made the program take forever
to shut down:

The problem was that the font system would try to load a glyph that's
0 pixels wide/tall (likely the space character), which SDL2 didn't
like, so it would fail to allocate the texture, causing
Backend_CreateSurface, and by extension Backend_CreateGlyph, to
return a NULL. Later, upon shutdown, the font system would pass this
NULL to Backend_FreeGlyph, causing NULL pointer dereferences that
make the program take forever to shut down.

Personally, I think passing NULLs to the backend is valid behaviour,
so I've added a bunch of sanity checks to make sure they're never
dereferenced.

--- a/src/Backends/Rendering/OpenGL2.cpp

+++ b/src/Backends/Rendering/OpenGL2.cpp

@@ -213,6 +213,9 @@

 void Backend_FreeSurface(Backend_Surface *surface)

 {

+	if (surface == NULL)

+		return;

+

 	glDeleteTextures(1, &surface->texture_id);

 	free(surface);

 }

@@ -219,6 +222,9 @@

 unsigned char* Backend_Lock(Backend_Surface *surface, unsigned int *pitch)

 {

+	if (surface == NULL)

+		return NULL;

+

 	surface->pixels = (unsigned char*)malloc(surface->width * surface->height * 3);

 	*pitch = surface->width * 3;

 	return surface->pixels;

@@ -226,6 +232,9 @@

 void Backend_Unlock(Backend_Surface *surface)

 {

+	if (surface == NULL)

+		return;

+

 	glBindTexture(GL_TEXTURE_2D, surface->texture_id);

 	glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, surface->width, surface->height, GL_RGB, GL_UNSIGNED_BYTE, surface->pixels);

 	free(surface->pixels);

@@ -256,6 +265,9 @@

 void Backend_Blit(Backend_Surface *source_surface, const RECT *rect, Backend_Surface *destination_surface, long x, long y, BOOL colour_key)

 {

+	if (source_surface == NULL || destination_surface == NULL)

+		return;

+

 	// Point our framebuffer to the destination texture

 	SetRenderTarget(destination_surface);

@@ -264,6 +276,9 @@

 void Backend_BlitToScreen(Backend_Surface *source_surface, const RECT *rect, long x, long y, BOOL colour_key)

 {

+	if (source_surface == NULL)

+		return;

+

 	// Point our framebuffer to the screen texture

 	SetRenderTarget(&framebuffer_surface);

@@ -292,6 +307,9 @@

 void Backend_ColourFill(Backend_Surface *surface, const RECT *rect, unsigned char red, unsigned char green, unsigned char blue)

 {

+	if (surface == NULL)

+		return;

+

 	// Point our framebuffer to the destination texture

 	SetRenderTarget(surface);

@@ -308,6 +326,9 @@

 void Backend_ScreenToSurface(Backend_Surface *surface, const RECT *rect)

 {

+	if (surface == NULL)

+		return;

+

 	// Point our framebuffer to the destination texture

 	SetRenderTarget(surface);

@@ -385,6 +406,9 @@

 void Backend_UnloadGlyph(Backend_Glyph *glyph)

 {

+	if (glyph == NULL)

+		return;

+

 	glDeleteTextures(1, &glyph->texture_id);

 	free(glyph);

 }

@@ -411,6 +435,9 @@

 void Backend_DrawGlyph(Backend_Surface *surface, Backend_Glyph *glyph, long x, long y, const unsigned char *colours)

 {

+	if (glyph == NULL || surface == NULL)

+		return;

+

 	// Point our framebuffer to the destination texture

 	SetRenderTarget(surface);

@@ -419,6 +446,9 @@

 void Backend_DrawGlyphToScreen(Backend_Glyph *glyph, long x, long y, const unsigned char *colours)

 {

+	if (glyph == NULL)

+		return;

+

 	// Point our framebuffer to the screen texture

 	SetRenderTarget(&framebuffer_surface);

--- a/src/Backends/Rendering/SDLSurface.cpp

+++ b/src/Backends/Rendering/SDLSurface.cpp

@@ -88,6 +88,9 @@

 void Backend_FreeSurface(Backend_Surface *surface)

 {

+	if (surface == NULL)

+		return;

+

 	SDL_FreeSurface(surface->sdl_surface);

 	free(surface);

 }

@@ -94,6 +97,9 @@

 unsigned char* Backend_Lock(Backend_Surface *surface, unsigned int *pitch)

 {

+	if (surface == NULL)

+		return NULL;

+

 	*pitch = surface->sdl_surface->pitch;

 	return (unsigned char*)surface->sdl_surface->pixels;

 }

@@ -105,6 +111,9 @@

 void Backend_Blit(Backend_Surface *source_surface, const RECT *rect, Backend_Surface *destination_surface, long x, long y, BOOL colour_key)

 {

+	if (source_surface == NULL || destination_surface == NULL)

+		return;

+

 	SDL_Rect source_rect;

 	RectToSDLRect(rect, &source_rect);

@@ -126,6 +135,9 @@

 void Backend_ColourFill(Backend_Surface *surface, const RECT *rect, unsigned char red, unsigned char green, unsigned char blue)

 {

+	if (surface == NULL)

+		return;

+

 	SDL_Rect destination_rect;

 	RectToSDLRect(rect, &destination_rect);

@@ -206,6 +218,9 @@

 void Backend_UnloadGlyph(Backend_Glyph *glyph)

 {

+	if (glyph == NULL)

+		return;

+

 	SDL_FreeSurface(glyph->sdl_surface);

 	free(glyph);

 }

@@ -212,6 +227,9 @@

 void Backend_DrawGlyph(Backend_Surface *surface, Backend_Glyph *glyph, long x, long y, const unsigned char *colours)

 {

+	if (glyph == NULL || surface == NULL)

+		return;

+

 	SDL_Rect rect;

 	rect.x = x;

 	rect.y = y;

--- a/src/Backends/Rendering/SDLTexture.cpp

+++ b/src/Backends/Rendering/SDLTexture.cpp

@@ -163,11 +163,15 @@

 void Backend_FreeSurface(Backend_Surface *surface)

 {

+	if (surface == NULL)

+		return;

+

 	if (surface->next)

 		surface->next->prev = surface->prev;

 	if (surface->prev)

 		surface->prev->next = surface->next;

+	SDL_DestroyTexture(surface->texture);

 	SDL_FreeSurface(surface->sdl_surface);

 	free(surface);

 }

@@ -174,6 +178,9 @@

 unsigned char* Backend_Lock(Backend_Surface *surface, unsigned int *pitch)

 {

+	if (surface == NULL)

+		return NULL;

+

 	*pitch = surface->sdl_surface->pitch;

 	return (unsigned char*)surface->sdl_surface->pixels;

 }

@@ -180,11 +187,17 @@

 void Backend_Unlock(Backend_Surface *surface)

 {

+	if (surface == NULL)

+		return;

+

 	surface->needs_syncing = TRUE;

 }

 void Backend_Blit(Backend_Surface *source_surface, const RECT *rect, Backend_Surface *destination_surface, long x, long y, BOOL colour_key)

 {

+	if (source_surface == NULL || destination_surface == NULL)

+		return;

+

 	if (source_surface->needs_syncing)

 	{

 		FlushSurface(source_surface);

@@ -209,6 +222,9 @@

 void Backend_BlitToScreen(Backend_Surface *source_surface, const RECT *rect, long x, long y, BOOL colour_key)

 {

+	if (source_surface == NULL)

+		return;

+

 	if (source_surface->needs_syncing)

 	{

 		FlushSurface(source_surface);

@@ -227,6 +243,9 @@

 void Backend_ColourFill(Backend_Surface *surface, const RECT *rect, unsigned char red, unsigned char green, unsigned char blue)

 {

+	if (surface == NULL)

+		return;

+

 	SDL_Rect sdl_rect;

 	RectToSDLRect(rect, &sdl_rect);

@@ -261,6 +280,9 @@

 void Backend_ScreenToSurface(Backend_Surface *surface, const RECT *rect)

 {

+	if (surface == NULL)

+		return;

+

 	SDL_Rect sdl_rect;

 	RectToSDLRect(rect, &sdl_rect);

@@ -362,6 +384,9 @@

 void Backend_UnloadGlyph(Backend_Glyph *glyph)

 {

+	if (glyph == NULL)

+		return;

+

 	Backend_FreeSurface(glyph->surface);

 	free(glyph);

 }

@@ -373,6 +398,9 @@

 	// colour-keys, so the next best thing is relying on the software fallback, but I don't like the idea

 	// of uploading textures to the GPU every time a glyph is drawn.

+	if (glyph == NULL || surface == NULL)

+		return;

+

 	RECT rect;

 	rect.left = 0;

 	rect.top = 0;

@@ -387,6 +415,9 @@

 void Backend_DrawGlyphToScreen(Backend_Glyph *glyph, long x, long y, const unsigned char *colours)

 {

+	if (glyph == NULL)

+		return;

+

 	RECT rect;

 	rect.left = 0;

 	rect.top = 0;

--- a/src/Backends/Rendering/Software.cpp

+++ b/src/Backends/Rendering/Software.cpp

@@ -93,6 +93,9 @@

 void Backend_FreeSurface(Backend_Surface *surface)

 {

+	if (surface == NULL)

+		return;

+

 	free(surface->pixels);

 	free(surface);

 }

@@ -99,6 +102,9 @@

 unsigned char* Backend_Lock(Backend_Surface *surface, unsigned int *pitch)

 {

+	if (surface == NULL)

+		return NULL;

+

 	*pitch = surface->pitch;

 	return surface->pixels;

 }

@@ -110,6 +116,9 @@

 void Backend_Blit(Backend_Surface *source_surface, const RECT *rect, Backend_Surface *destination_surface, long x, long y, BOOL colour_key)

 {

+	if (source_surface == NULL || destination_surface == NULL)

+		return;

+

 	RECT rect_clamped;

 	rect_clamped.left = rect->left;

@@ -195,6 +204,9 @@

 void Backend_ColourFill(Backend_Surface *surface, const RECT *rect, unsigned char red, unsigned char green, unsigned char blue)

 {

+	if (surface == NULL)

+		return;

+

 	RECT rect_clamped;

 	rect_clamped.left = rect->left;

@@ -356,6 +368,9 @@

 void Backend_UnloadGlyph(Backend_Glyph *glyph)

 {

+	if (glyph == NULL)

+		return;

+

 	free(glyph->pixels);

 	free(glyph);

 }

@@ -362,6 +377,9 @@

 void Backend_DrawGlyph(Backend_Surface *surface, Backend_Glyph *glyph, long x, long y, const unsigned char *colours)

 {

+	if (glyph == NULL || surface == NULL)

+		return;

+

 	switch (glyph->pixel_mode)

 	{

 		case FONT_PIXEL_MODE_LCD: