SDL: Fix property cleanup callback not being called on error (#9663)

From 56feecc17d77f7840cc8dd20c87e70b224364bab Mon Sep 17 00:00:00 2001
From: Susko3 <[EMAIL REDACTED]>
Date: Mon, 6 May 2024 23:50:28 +0200
Subject: [PATCH] Fix property cleanup callback not being called on error
 (#9663)

The documentation for `SDL_SetPropertyWithCleanup` mentions that the cleanup function
is called upon failure. But this wasn't working in the code.
---
 src/SDL_properties.c                               | 14 ++++++++++----
 .../mediafoundation/SDL_camera_mediafoundation.c   |  3 ---
 src/video/SDL_video.c                              |  5 ++++-
 test/testautomation_properties.c                   |  2 +-
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/SDL_properties.c b/src/SDL_properties.c
index fd209c456fa17..1a094caacc87e 100644
--- a/src/SDL_properties.c
+++ b/src/SDL_properties.c
@@ -305,11 +305,11 @@ static int SDL_PrivateSetProperty(SDL_PropertiesID props, const char *name, SDL_
     int result = 0;
 
     if (!props) {
-        SDL_FreePropertyWithCleanup(NULL, property, NULL, SDL_FALSE);
+        SDL_FreePropertyWithCleanup(NULL, property, NULL, SDL_TRUE);
         return SDL_InvalidParamError("props");
     }
     if (!name || !*name) {
-        SDL_FreePropertyWithCleanup(NULL, property, NULL, SDL_FALSE);
+        SDL_FreePropertyWithCleanup(NULL, property, NULL, SDL_TRUE);
         return SDL_InvalidParamError("name");
     }
 
@@ -318,7 +318,7 @@ static int SDL_PrivateSetProperty(SDL_PropertiesID props, const char *name, SDL_
     SDL_UnlockMutex(SDL_properties_lock);
 
     if (!properties) {
-        SDL_FreePropertyWithCleanup(NULL, property, NULL, SDL_FALSE);
+        SDL_FreePropertyWithCleanup(NULL, property, NULL, SDL_TRUE);
         return SDL_InvalidParamError("props");
     }
 
@@ -328,7 +328,7 @@ static int SDL_PrivateSetProperty(SDL_PropertiesID props, const char *name, SDL_
         if (property) {
             char *key = SDL_strdup(name);
             if (!SDL_InsertIntoHashTable(properties->props, key, property)) {
-                SDL_FreePropertyWithCleanup(key, property, NULL, SDL_FALSE);
+                SDL_FreePropertyWithCleanup(key, property, NULL, SDL_TRUE);
                 result = -1;
             }
         }
@@ -343,11 +343,17 @@ int SDL_SetPropertyWithCleanup(SDL_PropertiesID props, const char *name, void *v
     SDL_Property *property;
 
     if (!value) {
+        if (cleanup) {
+            cleanup(userdata, value);
+        }
         return SDL_ClearProperty(props, name);
     }
 
     property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
     if (!property) {
+        if (cleanup) {
+            cleanup(userdata, value);
+        }
         SDL_FreePropertyWithCleanup(NULL, property, NULL, SDL_FALSE);
         return -1;
     }
diff --git a/src/camera/mediafoundation/SDL_camera_mediafoundation.c b/src/camera/mediafoundation/SDL_camera_mediafoundation.c
index 86d500592a3e9..70b6c946f9bf1 100644
--- a/src/camera/mediafoundation/SDL_camera_mediafoundation.c
+++ b/src/camera/mediafoundation/SDL_camera_mediafoundation.c
@@ -274,7 +274,6 @@ static int MEDIAFOUNDATION_AcquireFrame(SDL_CameraDevice *device, SDL_Surface *f
                 frame->pixels = pixels;
                 frame->pitch = (int) pitch;
                 if (SDL_SetPropertyWithCleanup(surfprops, PROP_SURFACE_IMFOBJS_POINTER, objs, CleanupIMF2DBuffer2, NULL) == -1) {
-                    CleanupIMF2DBuffer2(NULL, objs);
                     retval = -1;
                 }
             }
@@ -287,7 +286,6 @@ static int MEDIAFOUNDATION_AcquireFrame(SDL_CameraDevice *device, SDL_Surface *f
                 frame->pixels = pixels;
                 frame->pitch = (int) pitch;
                 if (SDL_SetPropertyWithCleanup(surfprops, PROP_SURFACE_IMFOBJS_POINTER, objs, CleanupIMF2DBuffer, NULL) == -1) {
-                    CleanupIMF2DBuffer(NULL, objs);
                     retval = -1;
                 }
             }
@@ -305,7 +303,6 @@ static int MEDIAFOUNDATION_AcquireFrame(SDL_CameraDevice *device, SDL_Surface *f
                 frame->pixels = pixels;
                 frame->pitch = (int) pitch;
                 if (SDL_SetPropertyWithCleanup(surfprops, PROP_SURFACE_IMFOBJS_POINTER, objs, CleanupIMFMediaBuffer, NULL) == -1) {
-                    CleanupIMFMediaBuffer(NULL, objs);
                     retval = -1;
                 }
             }
diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c
index 18e4d255abed3..403f488f60d21 100644
--- a/src/video/SDL_video.c
+++ b/src/video/SDL_video.c
@@ -321,7 +321,10 @@ static int SDL_CreateWindowTexture(SDL_VideoDevice *_this, SDL_Window *window, S
             SDL_DestroyRenderer(renderer);
             return -1;
         }
-        SDL_SetPropertyWithCleanup(props, SDL_PROP_WINDOW_TEXTUREDATA_POINTER, data, SDL_CleanupWindowTextureData, NULL);
+        if (SDL_SetPropertyWithCleanup(props, SDL_PROP_WINDOW_TEXTUREDATA_POINTER, data, SDL_CleanupWindowTextureData, NULL) < 0) {
+            SDL_DestroyRenderer(renderer);
+            return -1;
+        }
 
         data->renderer = renderer;
     }
diff --git a/test/testautomation_properties.c b/test/testautomation_properties.c
index 83f8f7ae198c2..8373cb36e49c8 100644
--- a/test/testautomation_properties.c
+++ b/test/testautomation_properties.c
@@ -293,7 +293,7 @@ static int properties_testCleanup(void *arg)
     SDLTest_AssertPass("Call to SDL_SetProperty(cleanup)");
     count = 0;
     SDL_SetPropertyWithCleanup(props, "a", "0", cleanup, &count);
-    SDL_SetPropertyWithCleanup(props, "a", NULL, cleanup, &count);
+    SDL_ClearProperty(props, "a");
     SDLTest_AssertCheck(count == 1,
         "Verify cleanup for deleting property, got %d, expected 1", count);