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.
This commit is contained in:
Clownacy 2019-07-24 23:34:16 +01:00
parent 679c6d0391
commit 145864cf2d
4 changed files with 97 additions and 0 deletions

View file

@ -213,12 +213,18 @@ Backend_Surface* Backend_CreateSurface(unsigned int width, unsigned int height)
void Backend_FreeSurface(Backend_Surface *surface)
{
if (surface == NULL)
return;
glDeleteTextures(1, &surface->texture_id);
free(surface);
}
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 @@ unsigned char* Backend_Lock(Backend_Surface *surface, unsigned int *pitch)
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 @@ static void BlitCommon(Backend_Surface *source_surface, const RECT *rect, long x
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_Blit(Backend_Surface *source_surface, const RECT *rect, Backend_Sur
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 @@ static void ColourFillCommon(const RECT *rect, unsigned char red, unsigned char
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_ColourFillToScreen(const RECT *rect, unsigned char red, unsigned ch
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 @@ Backend_Glyph* Backend_LoadGlyph(const unsigned char *pixels, unsigned int width
void Backend_UnloadGlyph(Backend_Glyph *glyph)
{
if (glyph == NULL)
return;
glDeleteTextures(1, &glyph->texture_id);
free(glyph);
}
@ -411,6 +435,9 @@ static void DrawGlyphCommon(Backend_Glyph *glyph, long x, long y, const unsigned
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_DrawGlyph(Backend_Surface *surface, Backend_Glyph *glyph, long x, l
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);

View file

@ -88,12 +88,18 @@ Backend_Surface* Backend_CreateSurface(unsigned int width, unsigned int height)
void Backend_FreeSurface(Backend_Surface *surface)
{
if (surface == NULL)
return;
SDL_FreeSurface(surface->sdl_surface);
free(surface);
}
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_Unlock(Backend_Surface *surface)
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_BlitToScreen(Backend_Surface *source_surface, const RECT *rect, lon
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,12 +218,18 @@ Backend_Glyph* Backend_LoadGlyph(const unsigned char *pixels, unsigned int width
void Backend_UnloadGlyph(Backend_Glyph *glyph)
{
if (glyph == NULL)
return;
SDL_FreeSurface(glyph->sdl_surface);
free(glyph);
}
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;

View file

@ -163,28 +163,41 @@ Backend_Surface* Backend_CreateSurface(unsigned int width, unsigned int height)
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);
}
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;
}
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_Blit(Backend_Surface *source_surface, const RECT *rect, Backend_Sur
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_BlitToScreen(Backend_Surface *source_surface, const RECT *rect, lon
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_ColourFillToScreen(const RECT *rect, unsigned char red, unsigned ch
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 @@ Backend_Glyph* Backend_LoadGlyph(const unsigned char *pixels, unsigned int width
void Backend_UnloadGlyph(Backend_Glyph *glyph)
{
if (glyph == NULL)
return;
Backend_FreeSurface(glyph->surface);
free(glyph);
}
@ -373,6 +398,9 @@ void Backend_DrawGlyph(Backend_Surface *surface, Backend_Glyph *glyph, long x, l
// 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_DrawGlyph(Backend_Surface *surface, Backend_Glyph *glyph, long x, l
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;

View file

@ -93,12 +93,18 @@ Backend_Surface* Backend_CreateSurface(unsigned int width, unsigned int height)
void Backend_FreeSurface(Backend_Surface *surface)
{
if (surface == NULL)
return;
free(surface->pixels);
free(surface);
}
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_Unlock(Backend_Surface *surface)
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_BlitToScreen(Backend_Surface *source_surface, const RECT *rect, lon
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,12 +368,18 @@ Backend_Glyph* Backend_LoadGlyph(const unsigned char *pixels, unsigned int width
void Backend_UnloadGlyph(Backend_Glyph *glyph)
{
if (glyph == NULL)
return;
free(glyph->pixels);
free(glyph);
}
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: