SDL_image: Fixed memory leak when saving PNG and JPG files if SDL_image isn't built with save support (cf019)

From cf019d527ec2dc016ea500a4c03cd75bc6f8018b Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Thu, 25 Jan 2024 13:42:32 -0800
Subject: [PATCH] Fixed memory leak when saving PNG and JPG files if SDL_image
 isn't built with save support

(cherry picked from commit d2c7e09053287b091f55db11e296ff12962a2194)
---
 src/IMG_jpg.c |  60 +++++-------
 src/IMG_png.c | 262 +++++++++++++++++++++++++-------------------------
 2 files changed, 152 insertions(+), 170 deletions(-)

diff --git a/src/IMG_jpg.c b/src/IMG_jpg.c
index 90d100c8..3feebdbf 100644
--- a/src/IMG_jpg.c
+++ b/src/IMG_jpg.c
@@ -477,7 +477,7 @@ static void jpeg_SDL_RW_dest(j_compress_ptr cinfo, SDL_RWops *ctx)
     dest->pub.free_in_buffer = OUTPUT_BUFFER_SIZE;
 }
 
-static int IMG_SaveJPG_RW_jpeglib(SDL_Surface *surface, SDL_RWops *dst, int freedst, int quality)
+static int IMG_SaveJPG_RW_jpeglib(SDL_Surface *surface, SDL_RWops *dst, int quality)
 {
     /* The JPEG library reads bytes in R,G,B order, so this is the right
      * encoding for either endianness */
@@ -486,22 +486,16 @@ static int IMG_SaveJPG_RW_jpeglib(SDL_Surface *surface, SDL_RWops *dst, int free
     struct my_error_mgr jerr;
     JSAMPROW row_pointer[1];
     SDL_Surface* jpeg_surface = surface;
-    int result = -1;
-
-    if (!dst) {
-        IMG_SetError("Passed NULL dst");
-        goto done;
-    }
 
     if (!IMG_Init(IMG_INIT_JPG)) {
-        goto done;
+        return -1;
     }
 
     /* Convert surface to format we can save */
     if (surface->format->format != jpg_format) {
         jpeg_surface = SDL_ConvertSurfaceFormat(surface, jpg_format, 0);
         if (!jpeg_surface) {
-            goto done;
+            return -1;
         }
     }
 
@@ -534,14 +528,7 @@ static int IMG_SaveJPG_RW_jpeglib(SDL_Surface *surface, SDL_RWops *dst, int free
     if (jpeg_surface != surface) {
         SDL_FreeSurface(jpeg_surface);
     }
-
-    result = 0;
-
-done:
-    if (freedst) {
-        SDL_RWclose(dst);
-    }
-    return result;
+    return 0;
 }
 
 #elif defined(USE_STBIMAGE)
@@ -687,7 +674,7 @@ static void IMG_SaveJPG_RW_tinyjpeg_callback(void* context, void* data, int size
     SDL_RWwrite((SDL_RWops*) context, data, 1, size);
 }
 
-static int IMG_SaveJPG_RW_tinyjpeg(SDL_Surface *surface, SDL_RWops *dst, int freedst, int quality)
+static int IMG_SaveJPG_RW_tinyjpeg(SDL_Surface *surface, SDL_RWops *dst, int quality)
 {
     /* The JPEG library reads bytes in R,G,B order, so this is the right
      * encoding for either endianness */
@@ -695,16 +682,11 @@ static int IMG_SaveJPG_RW_tinyjpeg(SDL_Surface *surface, SDL_RWops *dst, int fre
     SDL_Surface* jpeg_surface = surface;
     int result = -1;
 
-    if (!dst) {
-        SDL_SetError("Passed NULL dst");
-        goto done;
-    }
-
     /* Convert surface to format we can save */
     if (surface->format->format != jpg_format) {
         jpeg_surface = SDL_ConvertSurfaceFormat(surface, jpg_format, 0);
         if (!jpeg_surface) {
-            goto done;
+            return -1;
         }
     }
 
@@ -735,11 +717,6 @@ static int IMG_SaveJPG_RW_tinyjpeg(SDL_Surface *surface, SDL_RWops *dst, int fre
     if (result < 0) {
         SDL_SetError("tinyjpeg error");
     }
-
-done:
-    if (freedst) {
-        SDL_RWclose(dst);
-    }
     return result;
 }
 
@@ -757,22 +734,31 @@ int IMG_SaveJPG(SDL_Surface *surface, const char *file, int quality)
 
 int IMG_SaveJPG_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst, int quality)
 {
+    int result = -1;
+
+    if (!dst) {
+        return IMG_SetError("Passed NULL dst");
+    }
+
 #if SDL_IMAGE_SAVE_JPG
 #ifdef USE_JPEGLIB
-    if ((IMG_Init(IMG_INIT_JPG) & IMG_INIT_JPG) != 0) {
-        if (IMG_SaveJPG_RW_jpeglib(surface, dst, freedst, quality) == 0) {
-            return 0;
-        }
+    if (result < 0) {
+        result = IMG_SaveJPG_RW_jpeglib(surface, dst, quality);
     }
 #endif
 
 #if defined(LOAD_JPG_DYNAMIC) || !defined(WANT_JPEGLIB)
-    return IMG_SaveJPG_RW_tinyjpeg(surface, dst, freedst, quality);
-#else
-    return -1;
+    if (result < 0) {
+        result = IMG_SaveJPG_RW_tinyjpeg(surface, dst, quality);
+    }
 #endif
 
 #else
-    return IMG_SetError("SDL_image built without JPEG save support");
+    result = IMG_SetError("SDL_image built without JPEG save support");
 #endif
+
+    if (freedst) {
+        SDL_RWclose(dst);
+    }
+    return result;
 }
diff --git a/src/IMG_png.c b/src/IMG_png.c
index a9648ab1..3f1f3a64 100644
--- a/src/IMG_png.c
+++ b/src/IMG_png.c
@@ -572,128 +572,119 @@ static void png_flush_data(png_structp png_ptr)
     (void)png_ptr;
 }
 
-static int IMG_SavePNG_RW_libpng(SDL_Surface *surface, SDL_RWops *dst, int freedst)
+static int IMG_SavePNG_RW_libpng(SDL_Surface *surface, SDL_RWops *dst)
 {
-    if (dst) {
-        png_structp png_ptr;
-        png_infop info_ptr;
-        png_colorp color_ptr = NULL;
-        Uint8 transparent_table[256];
-        SDL_Surface *source = surface;
-        SDL_Palette *palette;
-        int png_color_type = PNG_COLOR_TYPE_RGB_ALPHA;
-
-        png_ptr = lib.png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
-        if (png_ptr == NULL) {
-            IMG_SetError("Couldn't allocate memory for PNG file or incompatible PNG dll");
-            return -1;
-        }
+    png_structp png_ptr;
+    png_infop info_ptr;
+    png_colorp color_ptr = NULL;
+    Uint8 transparent_table[256];
+    SDL_Surface *source = surface;
+    SDL_Palette *palette;
+    int png_color_type = PNG_COLOR_TYPE_RGB_ALPHA;
 
-        info_ptr = lib.png_create_info_struct(png_ptr);
-        if (info_ptr == NULL) {
-            lib.png_destroy_write_struct(&png_ptr, NULL);
-            IMG_SetError("Couldn't create image information for PNG file");
-            return -1;
-        }
+    if (!IMG_Init(IMG_INIT_PNG)) {
+        return -1;
+    }
+
+    png_ptr = lib.png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
+    if (png_ptr == NULL) {
+        return IMG_SetError("Couldn't allocate memory for PNG file or incompatible PNG dll");
+    }
+
+    info_ptr = lib.png_create_info_struct(png_ptr);
+    if (info_ptr == NULL) {
+        lib.png_destroy_write_struct(&png_ptr, NULL);
+        return IMG_SetError("Couldn't create image information for PNG file");
+    }
 #ifdef PNG_SETJMP_SUPPORTED
 #ifndef LIBPNG_VERSION_12
-        if (setjmp(*lib.png_set_longjmp_fn(png_ptr, longjmp, sizeof (jmp_buf))))
+    if (setjmp(*lib.png_set_longjmp_fn(png_ptr, longjmp, sizeof (jmp_buf))))
 #else
-        if (setjmp(png_ptr->jmpbuf))
+    if (setjmp(png_ptr->jmpbuf))
 #endif
 #endif
+    {
+        lib.png_destroy_write_struct(&png_ptr, &info_ptr);
+        return IMG_SetError("Error writing the PNG file.");
+    }
+
+    palette = surface->format->palette;
+    if (palette) {
+        const int ncolors = palette->ncolors;
+        int i;
+        int last_transparent = -1;
+
+        color_ptr = (png_colorp)SDL_malloc(sizeof(png_color) * ncolors);
+        if (color_ptr == NULL)
         {
             lib.png_destroy_write_struct(&png_ptr, &info_ptr);
-            IMG_SetError("Error writing the PNG file.");
-            return -1;
+            return IMG_SetError("Couldn't create palette for PNG file");
         }
-
-        palette = surface->format->palette;
-        if (palette) {
-            const int ncolors = palette->ncolors;
-            int i;
-            int last_transparent = -1;
-
-            color_ptr = (png_colorp)SDL_malloc(sizeof(png_color) * ncolors);
-            if (color_ptr == NULL)
-            {
-                lib.png_destroy_write_struct(&png_ptr, &info_ptr);
-                IMG_SetError("Couldn't create palette for PNG file");
-                return -1;
-            }
-            for (i = 0; i < ncolors; i++) {
-                color_ptr[i].red = palette->colors[i].r;
-                color_ptr[i].green = palette->colors[i].g;
-                color_ptr[i].blue = palette->colors[i].b;
-                if (palette->colors[i].a != 255) {
-                    last_transparent = i;
-                }
+        for (i = 0; i < ncolors; i++) {
+            color_ptr[i].red = palette->colors[i].r;
+            color_ptr[i].green = palette->colors[i].g;
+            color_ptr[i].blue = palette->colors[i].b;
+            if (palette->colors[i].a != 255) {
+                last_transparent = i;
             }
-            lib.png_set_PLTE(png_ptr, info_ptr, color_ptr, ncolors);
-            png_color_type = PNG_COLOR_TYPE_PALETTE;
+        }
+        lib.png_set_PLTE(png_ptr, info_ptr, color_ptr, ncolors);
+        png_color_type = PNG_COLOR_TYPE_PALETTE;
 
-            if (last_transparent >= 0) {
-                for (i = 0; i <= last_transparent; ++i) {
-                    transparent_table[i] = palette->colors[i].a;
-                }
-                lib.png_set_tRNS(png_ptr, info_ptr, transparent_table, last_transparent + 1, NULL);
+        if (last_transparent >= 0) {
+            for (i = 0; i <= last_transparent; ++i) {
+                transparent_table[i] = palette->colors[i].a;
             }
+            lib.png_set_tRNS(png_ptr, info_ptr, transparent_table, last_transparent + 1, NULL);
         }
-        else if (surface->format->format == SDL_PIXELFORMAT_RGB24) {
-            /* If the surface is exactly the right RGB format it is just passed through */
-            png_color_type = PNG_COLOR_TYPE_RGB;
-        }
-        else if (!SDL_ISPIXELFORMAT_ALPHA(surface->format->format)) {
-            /* If the surface is not exactly the right RGB format but does not have alpha
-               information, it should be converted to RGB24 before being passed through */
-            png_color_type = PNG_COLOR_TYPE_RGB;
-            source = SDL_ConvertSurfaceFormat(surface, SDL_PIXELFORMAT_RGB24, 0);
-        }
-        else if (surface->format->format != png_format) {
-            /* Otherwise, (surface has alpha data), and it is not in the exact right
-               format , so it should be converted to that */
-            source = SDL_ConvertSurfaceFormat(surface, png_format, 0);
-        }
-
-        lib.png_set_write_fn(png_ptr, dst, png_write_data, png_flush_data);
-
-        lib.png_set_IHDR(png_ptr, info_ptr, surface->w, surface->h,
-                         8, png_color_type, PNG_INTERLACE_NONE,
-                         PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
+    }
+    else if (surface->format->format == SDL_PIXELFORMAT_RGB24) {
+        /* If the surface is exactly the right RGB format it is just passed through */
+        png_color_type = PNG_COLOR_TYPE_RGB;
+    }
+    else if (!SDL_ISPIXELFORMAT_ALPHA(surface->format->format)) {
+        /* If the surface is not exactly the right RGB format but does not have alpha
+           information, it should be converted to RGB24 before being passed through */
+        png_color_type = PNG_COLOR_TYPE_RGB;
+        source = SDL_ConvertSurfaceFormat(surface, SDL_PIXELFORMAT_RGB24, 0);
+    }
+    else if (surface->format->format != png_format) {
+        /* Otherwise, (surface has alpha data), and it is not in the exact right
+           format , so it should be converted to that */
+        source = SDL_ConvertSurfaceFormat(surface, png_format, 0);
+    }
 
-        if (source) {
-            png_bytep *row_pointers;
-            int row;
+    lib.png_set_write_fn(png_ptr, dst, png_write_data, png_flush_data);
 
-            row_pointers = (png_bytep *) SDL_malloc(sizeof(png_bytep) * source->h);
-            if (!row_pointers) {
-                SDL_free(color_ptr);
-                lib.png_destroy_write_struct(&png_ptr, &info_ptr);
-                IMG_SetError("Out of memory");
-                return -1;
-            }
-            for (row = 0; row < (int)source->h; row++) {
-                row_pointers[row] = (png_bytep) (Uint8 *) source->pixels + row * source->pitch;
-            }
+    lib.png_set_IHDR(png_ptr, info_ptr, surface->w, surface->h,
+                     8, png_color_type, PNG_INTERLACE_NONE,
+                     PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
 
-            lib.png_set_rows(png_ptr, info_ptr, row_pointers);
-            lib.png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);
+    if (source) {
+        png_bytep *row_pointers;
+        int row;
 
-            SDL_free(row_pointers);
-            if (source != surface) {
-                SDL_FreeSurface(source);
-            }
-        }
-        lib.png_destroy_write_struct(&png_ptr, &info_ptr);
-        if (color_ptr) {
+        row_pointers = (png_bytep *) SDL_malloc(sizeof(png_bytep) * source->h);
+        if (!row_pointers) {
             SDL_free(color_ptr);
+            lib.png_destroy_write_struct(&png_ptr, &info_ptr);
+            return IMG_SetError("Out of memory");
         }
-        if (freedst) {
-            SDL_RWclose(dst);
+        for (row = 0; row < (int)source->h; row++) {
+            row_pointers[row] = (png_bytep) (Uint8 *) source->pixels + row * source->pitch;
         }
-    } else {
-        IMG_SetError("Passed NULL dst");
-        return -1;
+
+        lib.png_set_rows(png_ptr, info_ptr, row_pointers);
+        lib.png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);
+
+        SDL_free(row_pointers);
+        if (source != surface) {
+            SDL_FreeSurface(source);
+        }
+    }
+    lib.png_destroy_write_struct(&png_ptr, &info_ptr);
+    if (color_ptr) {
+        SDL_free(color_ptr);
     }
     return 0;
 }
@@ -720,36 +711,32 @@ static int IMG_SavePNG_RW_libpng(SDL_Surface *surface, SDL_RWops *dst, int freed
 #define MINIZ_SDL_NOUNUSED
 #include "miniz.h"
 
-static int IMG_SavePNG_RW_miniz(SDL_Surface *surface, SDL_RWops *dst, int freedst)
+static int IMG_SavePNG_RW_miniz(SDL_Surface *surface, SDL_RWops *dst)
 {
+    size_t size = 0;
+    void *png = NULL;
     int result = -1;
 
-    if (dst) {
-        size_t size = 0;
-        void *png = NULL;
+    if (!dst) {
+        return IMG_SetError("Passed NULL dst");
+    }
 
-        if (surface->format->format == png_format) {
-            png = tdefl_write_image_to_png_file_in_memory(surface->pixels, surface->w, surface->h, surface->format->BytesPerPixel, surface->pitch, &size);
-        } else {
-            SDL_Surface *cvt = SDL_ConvertSurfaceFormat(surface, png_format, 0);
-            if (cvt) {
-                png = tdefl_write_image_to_png_file_in_memory(cvt->pixels, cvt->w, cvt->h, cvt->format->BytesPerPixel, cvt->pitch, &size);
-                SDL_FreeSurface(cvt);
-            }
-        }
-        if (png) {
-            if (SDL_RWwrite(dst, png, size, 1)) {
-                result = 0;
-            }
-            mz_free(png); /* calls SDL_free() */
-        } else {
-            IMG_SetError("Failed to convert and save image");
+    if (surface->format->format == png_format) {
+        png = tdefl_write_image_to_png_file_in_memory(surface->pixels, surface->w, surface->h, surface->format->BytesPerPixel, surface->pitch, &size);
+    } else {
+        SDL_Surface *cvt = SDL_ConvertSurfaceFormat(surface, png_format, 0);
+        if (cvt) {
+            png = tdefl_write_image_to_png_file_in_memory(cvt->pixels, cvt->w, cvt->h, cvt->format->BytesPerPixel, cvt->pitch, &size);
+            SDL_FreeSurface(cvt);
         }
-        if (freedst) {
-            SDL_RWclose(dst);
+    }
+    if (png) {
+        if (SDL_RWwrite(dst, png, size, 1)) {
+            result = 0;
         }
+        mz_free(png); /* calls SDL_free() */
     } else {
-        IMG_SetError("Passed NULL dst");
+        return IMG_SetError("Failed to convert and save image");
     }
     return result;
 }
@@ -769,22 +756,31 @@ int IMG_SavePNG(SDL_Surface *surface, const char *file)
 
 int IMG_SavePNG_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst)
 {
+    int result = -1;
+
+    if (!dst) {
+        return IMG_SetError("Passed NULL dst");
+    }
+
 #if SDL_IMAGE_SAVE_PNG
 #ifdef USE_LIBPNG
-    if ((IMG_Init(IMG_INIT_PNG) & IMG_INIT_PNG) != 0) {
-        if (IMG_SavePNG_RW_libpng(surface, dst, freedst) == 0) {
-            return 0;
-        }
+    if (result < 0) {
+        result = IMG_SavePNG_RW_libpng(surface, dst);
     }
 #endif
+
 #if defined(LOAD_PNG_DYNAMIC) || !defined(WANT_LIBPNG)
-    return IMG_SavePNG_RW_miniz(surface, dst, freedst);
-#else
-    return -1;
+    if (result < 0) {
+        result = IMG_SavePNG_RW_miniz(surface, dst);
+    }
 #endif
 
 #else
-    return IMG_SetError("SDL_image built without PNG save support");
+    result = IMG_SetError("SDL_image built without PNG save support");
+#endif
 
-#endif /* SDL_IMAGE_SAVE_PNG */
+    if (freedst) {
+        SDL_RWclose(dst);
+    }
+    return result;
 }