From a555a69fc6ce0369fc8e12b01734c119bbbd78d0 Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Fri, 2 Oct 2026 00:01:08 -0700
Subject: [PATCH] Don't do additional allocations when setting a property to an
existing value
Fixes https://github.com/libsdl-org/SDL/issues/16428
---
src/SDL_properties.c | 351 ++++++++++++++++++++++++++++++++-----------
1 file changed, 260 insertions(+), 91 deletions(-)
diff --git a/src/SDL_properties.c b/src/SDL_properties.c
index 4c67b5d0b5bec..467964e48ced0 100644
--- a/src/SDL_properties.c
+++ b/src/SDL_properties.c
@@ -55,23 +55,36 @@ static SDL_AtomicU32 SDL_last_properties_id;
static SDL_AtomicU32 SDL_global_properties;
-static void SDL_FreePropertyWithCleanup(const void *key, const void *value, void *data, bool cleanup)
+static void SDL_FreePropertyContents(SDL_Property *property, bool cleanup)
{
- SDL_Property *property = (SDL_Property *)value;
- if (property) {
- switch (property->type) {
- case SDL_PROPERTY_TYPE_POINTER:
- if (property->cleanup && cleanup) {
- property->cleanup(property->userdata, property->value.pointer_value);
- }
- break;
- case SDL_PROPERTY_TYPE_STRING:
+ switch (property->type) {
+ case SDL_PROPERTY_TYPE_POINTER:
+ if (property->cleanup && cleanup) {
+ property->cleanup(property->userdata, property->value.pointer_value);
+ }
+ property->cleanup = NULL;
+ property->userdata = NULL;
+ break;
+ case SDL_PROPERTY_TYPE_STRING:
+ if (property->value.string_value) {
SDL_free(property->value.string_value);
- break;
- default:
- break;
+ property->value.string_value = NULL;
}
+ break;
+ default:
+ break;
+ }
+ if (property->string_storage) {
SDL_free(property->string_storage);
+ property->string_storage = NULL;
+ }
+}
+
+static void SDL_FreePropertyWithCleanup(const void *key, const void *value, void *data, bool cleanup)
+{
+ SDL_Property *property = (SDL_Property *)value;
+ if (property) {
+ SDL_FreePropertyContents(property, cleanup);
}
SDL_free((void *)key);
SDL_free((void *)value);
@@ -328,32 +341,59 @@ void SDL_UnlockProperties(SDL_PropertiesID props)
SDL_UnlockMutex(properties->lock);
}
-static bool SDL_PrivateSetProperty(SDL_PropertiesID props, const char *name, SDL_Property *property)
+bool SDL_SetPointerPropertyWithCleanup(SDL_PropertiesID props, const char *name, void *value, SDL_CleanupPropertyCallback cleanup, void *userdata)
{
SDL_Properties *properties = NULL;
bool result = true;
+ if (!value) {
+ result = SDL_ClearProperty(props, name);
+ goto done;
+ }
+
CHECK_PARAM(!props) {
- SDL_FreePropertyWithCleanup(NULL, property, NULL, true);
- return SDL_InvalidParamError("props");
+ result = SDL_InvalidParamError("props");
+ goto done;
}
CHECK_PARAM(!name || !*name) {
- SDL_FreePropertyWithCleanup(NULL, property, NULL, true);
- return SDL_InvalidParamError("name");
+ result = SDL_InvalidParamError("name");
+ goto done;
}
SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
CHECK_PARAM(!properties) {
- SDL_FreePropertyWithCleanup(NULL, property, NULL, true);
- return SDL_InvalidParamError("props");
+ result = SDL_InvalidParamError("props");
+ goto done;
}
SDL_LockMutex(properties->lock);
{
- SDL_RemoveFromHashTable(properties->props, name);
- if (property) {
+ SDL_Property *existing;
+ if (SDL_FindInHashTable(properties->props, name, (const void **)&existing)) {
+ if (existing->type == SDL_PROPERTY_TYPE_POINTER &&
+ existing->value.pointer_value == value &&
+ existing->cleanup == cleanup &&
+ existing->userdata == userdata) {
+ // The value is already set
+ } else {
+ SDL_FreePropertyContents(existing, true);
+
+ existing->type = SDL_PROPERTY_TYPE_POINTER;
+ existing->value.pointer_value = value;
+ existing->cleanup = cleanup;
+ existing->userdata = userdata;
+ }
+ } else {
char *key = SDL_strdup(name);
- if (!key || !SDL_InsertIntoHashTable(properties->props, key, property, false)) {
+ SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
+ if (property) {
+ property->type = SDL_PROPERTY_TYPE_POINTER;
+ property->value.pointer_value = value;
+ property->cleanup = cleanup;
+ property->userdata = userdata;
+ }
+ if (!key || !property ||
+ !SDL_InsertIntoHashTable(properties->props, key, property, false)) {
SDL_FreePropertyWithCleanup(key, property, NULL, true);
result = false;
}
@@ -361,50 +401,16 @@ static bool SDL_PrivateSetProperty(SDL_PropertiesID props, const char *name, SDL
}
SDL_UnlockMutex(properties->lock);
- return result;
-}
-
-bool SDL_SetPointerPropertyWithCleanup(SDL_PropertiesID props, const char *name, void *value, SDL_CleanupPropertyCallback cleanup, void *userdata)
-{
- SDL_Property *property;
-
- if (!value) {
- if (cleanup) {
- cleanup(userdata, value);
- }
- return SDL_ClearProperty(props, name);
+done:
+ if (!result && cleanup) {
+ cleanup(userdata, value);
}
-
- property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
- if (!property) {
- if (cleanup) {
- cleanup(userdata, value);
- }
- SDL_FreePropertyWithCleanup(NULL, property, NULL, false);
- return false;
- }
- property->type = SDL_PROPERTY_TYPE_POINTER;
- property->value.pointer_value = value;
- property->cleanup = cleanup;
- property->userdata = userdata;
- return SDL_PrivateSetProperty(props, name, property);
+ return result;
}
bool SDL_SetPointerProperty(SDL_PropertiesID props, const char *name, void *value)
{
- SDL_Property *property;
-
- if (!value) {
- return SDL_ClearProperty(props, name);
- }
-
- property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
- if (!property) {
- return false;
- }
- property->type = SDL_PROPERTY_TYPE_POINTER;
- property->value.pointer_value = value;
- return SDL_PrivateSetProperty(props, name, property);
+ return SDL_SetPointerPropertyWithCleanup(props, name, value, NULL, NULL);
}
static void SDLCALL CleanupFreeableProperty(void *userdata, void *value)
@@ -431,56 +437,199 @@ bool SDL_SetSurfaceProperty(SDL_PropertiesID props, const char *name, SDL_Surfac
bool SDL_SetStringProperty(SDL_PropertiesID props, const char *name, const char *value)
{
- SDL_Property *property;
+ SDL_Properties *properties = NULL;
+ bool result = true;
if (!value) {
return SDL_ClearProperty(props, name);
}
- property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
- if (!property) {
- return false;
+ CHECK_PARAM(!props) {
+ return SDL_InvalidParamError("props");
}
- property->type = SDL_PROPERTY_TYPE_STRING;
- property->value.string_value = SDL_strdup(value);
- if (!property->value.string_value) {
- SDL_free(property);
- return false;
+ CHECK_PARAM(!name || !*name) {
+ return SDL_InvalidParamError("name");
}
- return SDL_PrivateSetProperty(props, name, property);
+
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ CHECK_PARAM(!properties) {
+ return SDL_InvalidParamError("props");
+ }
+
+ SDL_LockMutex(properties->lock);
+ {
+ SDL_Property *existing;
+ if (SDL_FindInHashTable(properties->props, name, (const void **)&existing)) {
+ if (existing->type == SDL_PROPERTY_TYPE_STRING &&
+ SDL_strcmp(existing->value.string_value, value) == 0) {
+ // The value is already set
+ } else {
+ char *string = SDL_strdup(value);
+ if (string) {
+ SDL_FreePropertyContents(existing, true);
+ existing->type = SDL_PROPERTY_TYPE_STRING;
+ existing->value.string_value = string;
+ } else {
+ result = false;
+ }
+ }
+ } else {
+ char *key = SDL_strdup(name);
+ SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
+ char *string = SDL_strdup(value);
+ if (property) {
+ property->type = SDL_PROPERTY_TYPE_STRING;
+ property->value.string_value = string;
+ }
+ if (!key || !property || !string ||
+ !SDL_InsertIntoHashTable(properties->props, key, property, false)) {
+ if (property) {
+ property->value.string_value = NULL;
+ }
+ SDL_free(string);
+ SDL_FreePropertyWithCleanup(key, property, NULL, true);
+ result = false;
+ }
+ }
+ }
+ SDL_UnlockMutex(properties->lock);
+
+ return result;
}
bool SDL_SetNumberProperty(SDL_PropertiesID props, const char *name, Sint64 value)
{
- SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
- if (!property) {
- return false;
+ SDL_Properties *properties = NULL;
+ bool result = true;
+
+ CHECK_PARAM(!props) {
+ return SDL_InvalidParamError("props");
}
- property->type = SDL_PROPERTY_TYPE_NUMBER;
- property->value.number_value = value;
- return SDL_PrivateSetProperty(props, name, property);
+ CHECK_PARAM(!name || !*name) {
+ return SDL_InvalidParamError("name");
+ }
+
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ CHECK_PARAM(!properties) {
+ return SDL_InvalidParamError("props");
+ }
+
+ SDL_LockMutex(properties->lock);
+ {
+ SDL_Property *existing;
+ if (SDL_FindInHashTable(properties->props, name, (const void **)&existing)) {
+ if (existing->type != SDL_PROPERTY_TYPE_NUMBER) {
+ SDL_FreePropertyContents(existing, true);
+ existing->type = SDL_PROPERTY_TYPE_NUMBER;
+ }
+ existing->value.number_value = value;
+ } else {
+ char *key = SDL_strdup(name);
+ SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
+ if (property) {
+ property->type = SDL_PROPERTY_TYPE_NUMBER;
+ property->value.number_value = value;
+ }
+ if (!key || !property ||
+ !SDL_InsertIntoHashTable(properties->props, key, property, false)) {
+ SDL_FreePropertyWithCleanup(key, property, NULL, true);
+ result = false;
+ }
+ }
+ }
+ SDL_UnlockMutex(properties->lock);
+
+ return result;
}
bool SDL_SetFloatProperty(SDL_PropertiesID props, const char *name, float value)
{
- SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
- if (!property) {
- return false;
+ SDL_Properties *properties = NULL;
+ bool result = true;
+
+ CHECK_PARAM(!props) {
+ return SDL_InvalidParamError("props");
+ }
+ CHECK_PARAM(!name || !*name) {
+ return SDL_InvalidParamError("name");
}
- property->type = SDL_PROPERTY_TYPE_FLOAT;
- property->value.float_value = value;
- return SDL_PrivateSetProperty(props, name, property);
+
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ CHECK_PARAM(!properties) {
+ return SDL_InvalidParamError("props");
+ }
+
+ SDL_LockMutex(properties->lock);
+ {
+ SDL_Property *existing;
+ if (SDL_FindInHashTable(properties->props, name, (const void **)&existing)) {
+ if (existing->type != SDL_PROPERTY_TYPE_FLOAT) {
+ SDL_FreePropertyContents(existing, true);
+ existing->type = SDL_PROPERTY_TYPE_FLOAT;
+ }
+ existing->value.float_value = value;
+ } else {
+ char *key = SDL_strdup(name);
+ SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
+ if (property) {
+ property->type = SDL_PROPERTY_TYPE_FLOAT;
+ property->value.float_value = value;
+ }
+ if (!key || !property ||
+ !SDL_InsertIntoHashTable(properties->props, key, property, false)) {
+ SDL_FreePropertyWithCleanup(key, property, NULL, true);
+ result = false;
+ }
+ }
+ }
+ SDL_UnlockMutex(properties->lock);
+
+ return result;
}
bool SDL_SetBooleanProperty(SDL_PropertiesID props, const char *name, bool value)
{
- SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
- if (!property) {
- return false;
+ SDL_Properties *properties = NULL;
+ bool result = true;
+
+ CHECK_PARAM(!props) {
+ return SDL_InvalidParamError("props");
+ }
+ CHECK_PARAM(!name || !*name) {
+ return SDL_InvalidParamError("name");
+ }
+
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ CHECK_PARAM(!properties) {
+ return SDL_InvalidParamError("props");
}
- property->type = SDL_PROPERTY_TYPE_BOOLEAN;
- property->value.boolean_value = value ? true : false;
- return SDL_PrivateSetProperty(props, name, property);
+
+ SDL_LockMutex(properties->lock);
+ {
+ SDL_Property *existing;
+ if (SDL_FindInHashTable(properties->props, name, (const void **)&existing)) {
+ if (existing->type != SDL_PROPERTY_TYPE_BOOLEAN) {
+ SDL_FreePropertyContents(existing, true);
+ existing->type = SDL_PROPERTY_TYPE_BOOLEAN;
+ }
+ existing->value.boolean_value = value;
+ } else {
+ char *key = SDL_strdup(name);
+ SDL_Property *property = (SDL_Property *)SDL_calloc(1, sizeof(*property));
+ if (property) {
+ property->type = SDL_PROPERTY_TYPE_BOOLEAN;
+ property->value.boolean_value = value;
+ }
+ if (!key || !property ||
+ !SDL_InsertIntoHashTable(properties->props, key, property, false)) {
+ SDL_FreePropertyWithCleanup(key, property, NULL, true);
+ result = false;
+ }
+ }
+ }
+ SDL_UnlockMutex(properties->lock);
+
+ return result;
}
bool SDL_HasProperty(SDL_PropertiesID props, const char *name)
@@ -743,7 +892,27 @@ bool SDL_GetBooleanProperty(SDL_PropertiesID props, const char *name, bool defau
bool SDL_ClearProperty(SDL_PropertiesID props, const char *name)
{
- return SDL_PrivateSetProperty(props, name, NULL);
+ SDL_Properties *properties = NULL;
+
+ CHECK_PARAM(!props) {
+ return SDL_InvalidParamError("props");
+ }
+ CHECK_PARAM(!name || !*name) {
+ return SDL_InvalidParamError("name");
+ }
+
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ CHECK_PARAM(!properties) {
+ return SDL_InvalidParamError("props");
+ }
+
+ SDL_LockMutex(properties->lock);
+ {
+ SDL_RemoveFromHashTable(properties->props, name);
+ }
+ SDL_UnlockMutex(properties->lock);
+
+ return true;
}
int SDL_GetNumProperties(SDL_PropertiesID props)