SDL: testcontroller: memory management cleanup

From 787786bdbc8382f3c4b6375b71ae696cdb54aef0 Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Sun, 16 Jul 2023 09:16:16 -0700
Subject: [PATCH] testcontroller: memory management cleanup

Also saved the guide button for last in the binding flow, since it may not be present on the controller or available from the OS
---
 test/gamepadutils.c   |  61 +++++++++++++++++++------
 test/testcontroller.c | 103 ++++++++++++++++++------------------------
 2 files changed, 92 insertions(+), 72 deletions(-)

diff --git a/test/gamepadutils.c b/test/gamepadutils.c
index c66e2bb09c16..01f28bea356c 100644
--- a/test/gamepadutils.c
+++ b/test/gamepadutils.c
@@ -2179,6 +2179,18 @@ static void FreeMappingParts(MappingParts *parts)
     SDL_zerop(parts);
 }
 
+/* Create a new mapping from the parts and free the old mapping and parts */
+static char *RecreateMapping(MappingParts *parts, char *mapping)
+{
+    char *new_mapping = JoinMapping(parts);
+    if (new_mapping) {
+        SDL_free(mapping);
+        mapping = new_mapping;
+    }
+    FreeMappingParts(parts);
+    return mapping;
+}
+
 static char *GetMappingValue(const char *mapping, const char *key)
 {
     int i;
@@ -2200,7 +2212,6 @@ static char *SetMappingValue(char *mapping, const char *key, const char *value)
 {
     MappingParts parts;
     int i;
-    char *new_mapping;
     char *new_key = NULL;
     char *new_value = NULL;
     char **new_keys = NULL;
@@ -2240,11 +2251,7 @@ static char *SetMappingValue(char *mapping, const char *key, const char *value)
     }
 
     if (result) {
-        new_mapping = JoinMapping(&parts);
-        if (new_mapping) {
-            SDL_free(mapping);
-            mapping = new_mapping;
-        }
+        mapping = RecreateMapping(&parts, mapping);
     } else {
         SDL_free(new_key);
         SDL_free(new_value);
@@ -2264,7 +2271,7 @@ static char *RemoveMappingKey(char *mapping, const char *key)
     if (i >= 0) {
         RemoveMappingValueAt(&parts, i);
     }
-    return JoinMapping(&parts);
+    return RecreateMapping(&parts, mapping);
 }
 
 SDL_bool MappingHasBindings(const char *mapping)
@@ -2323,20 +2330,48 @@ char *GetMappingName(const char *mapping)
 char *SetMappingName(char *mapping, const char *name)
 {
     MappingParts parts;
-    char *new_name;
-    char *new_mapping;
+    char *new_name, *spot;
+    size_t length;
+
+    if (!name) {
+        return mapping;
+    }
+
+    /* Remove any leading whitespace */
+    while (*name && SDL_isspace(*name)) {
+        ++name;
+    }
 
     new_name = SDL_strdup(name);
     if (!new_name) {
         return mapping;
     }
 
+    /* Remove any commas, which are field separators in the mapping */
+    length = SDL_strlen(new_name);
+    while ((spot = SDL_strchr(new_name, ',')) != NULL) {
+        SDL_memmove(spot, spot + 1, length - (spot - new_name) - 1);
+        --length;
+    }
+
+    /* Remove any trailing whitespace */
+    while (length > 0 && SDL_isspace(new_name[length - 1])) {
+        --length;
+    }
+
+    /* See if we have anything left */
+    if (length == 0) {
+        SDL_free(new_name);
+        return mapping;
+    }
+
+    /* null terminate to cut off anything we've trimmed */
+    new_name[length] = '\0';
+
     SplitMapping(mapping, &parts);
     SDL_free(parts.name);
     parts.name = new_name;
-    new_mapping = JoinMapping(&parts);
-    FreeMappingParts(&parts);
-    return new_mapping;
+    return RecreateMapping(&parts, mapping);
 }
 
 char *GetMappingType(const char *mapping)
@@ -2449,5 +2484,5 @@ char *ClearMappingBinding(char *mapping, const char *binding)
             RemoveMappingValueAt(&parts, i);
         }
     }
-    return JoinMapping(&parts);
+    return RecreateMapping(&parts, mapping);
 }
diff --git a/test/testcontroller.c b/test/testcontroller.c
index 1dbb3bd01605..1c1a51982398 100644
--- a/test/testcontroller.c
+++ b/test/testcontroller.c
@@ -209,18 +209,10 @@ static int StandardizeAxisValue(int nValue)
     }
 }
 
-static void SetGamepadMapping(char *mapping)
+static void SetAndFreeGamepadMapping(char *mapping)
 {
-    /* Make sure the mapping has a valid name */
-    if (!MappingHasName(mapping)) {
-        SetMappingName(mapping, SDL_GetJoystickName(controller->joystick));
-    }
-
-    SDL_free(controller->mapping);
-    controller->mapping = mapping;
-    controller->has_bindings = MappingHasBindings(mapping);
-
     SDL_SetGamepadMapping(controller->id, mapping);
+    SDL_free(mapping);
 }
 
 static void SetCurrentBindingElement(int element)
@@ -264,8 +256,8 @@ static void SetNextBindingElement()
         SDL_GAMEPAD_BUTTON_DPAD_DOWN,
         SDL_GAMEPAD_BUTTON_DPAD_LEFT,
         SDL_GAMEPAD_BUTTON_BACK,
-        SDL_GAMEPAD_BUTTON_GUIDE,
         SDL_GAMEPAD_BUTTON_START,
+        SDL_GAMEPAD_BUTTON_GUIDE,
         SDL_GAMEPAD_BUTTON_MISC1,
         SDL_GAMEPAD_ELEMENT_INVALID,
 
@@ -304,7 +296,11 @@ static void CommitBindingElement(const char *binding, SDL_bool force)
         return;
     }
 
-    mapping = controller->mapping;
+    if (controller->mapping) {
+        mapping = SDL_strdup(controller->mapping);
+    } else {
+        mapping = NULL;
+    }
 
     /* If the controller generates multiple events for a single element, pick the best one */
     if (!force && binding_advance_time) {
@@ -360,7 +356,7 @@ static void CommitBindingElement(const char *binding, SDL_bool force)
 
     mapping = ClearMappingBinding(mapping, binding);
     mapping = SetElementBinding(mapping, binding_element, binding);
-    SetGamepadMapping(mapping);
+    SetAndFreeGamepadMapping(mapping);
 
     if (force) {
         SetNextBindingElement();
@@ -411,43 +407,22 @@ static void SetDisplayMode(ControllerDisplayMode mode)
 
 static void CancelMapping(void)
 {
-    if (backup_mapping) {
-        SetGamepadMapping(backup_mapping);
-        backup_mapping = NULL;
-    }
+    SetAndFreeGamepadMapping(backup_mapping);
+    backup_mapping = NULL;
+
     SetDisplayMode(CONTROLLER_MODE_TESTING);
 }
 
 static void ClearMapping(void)
 {
-    SetGamepadMapping(NULL);
+    SetAndFreeGamepadMapping(NULL);
     SetCurrentBindingElement(SDL_GAMEPAD_ELEMENT_INVALID);
 }
 
 static void CopyMapping(void)
 {
     if (controller && controller->mapping) {
-        char *mapping = controller->mapping;
-        char *wildcard = SDL_strchr(mapping, '*');
-        const char *name = SDL_GetGamepadName(controller->gamepad);
-        if (wildcard && name && *name) {
-            char *text;
-            size_t size;
-
-            /* Personalize the mapping for this controller */
-            *wildcard++ = '\0';
-            size = SDL_strlen(mapping) + SDL_strlen(name) + SDL_strlen(wildcard) + 1;
-            text = SDL_malloc(size);
-            if (!text) {
-                return;
-            }
-            SDL_snprintf(text, size, "%s%s%s", mapping, name, wildcard);
-            SDL_SetClipboardText(text);
-            SDL_free(text);
-            *wildcard = '*';
-        } else {
-            SDL_SetClipboardText(mapping);
-        }
+        SDL_SetClipboardText(controller->mapping);
     }
 }
 
@@ -457,7 +432,7 @@ static void PasteMapping(void)
         char *mapping = SDL_GetClipboardText();
         if (MappingHasBindings(mapping)) {
             CancelBinding();
-            SetGamepadMapping(mapping);
+            SetAndFreeGamepadMapping(mapping);
         } else {
             /* Not a valid mapping, ignore it */
             SDL_free(mapping);
@@ -584,6 +559,32 @@ static void SetController(SDL_JoystickID id)
     }
 }
 
+static void HandleGamepadRemapped(SDL_JoystickID id)
+{
+    char *mapping;
+    int i = FindController(id);
+
+    if (i < 0) {
+        return;
+    }
+
+    if (!controllers[i].gamepad) {
+        controllers[i].gamepad = SDL_OpenGamepad(id);
+    }
+
+    /* Get the current mapping */
+    mapping = SDL_GetGamepadMapping(controllers[i].gamepad);
+
+    /* Make sure the mapping has a valid name */
+    if (mapping && !MappingHasName(mapping)) {
+        mapping = SetMappingName(mapping, SDL_GetJoystickName(controllers[i].joystick));
+    }
+
+    SDL_free(controllers[i].mapping);
+    controllers[i].mapping = mapping;
+    controllers[i].has_bindings = MappingHasBindings(mapping);
+}
+
 static void AddController(SDL_JoystickID id, SDL_bool verbose)
 {
     Controller *new_controllers;
@@ -620,8 +621,6 @@ static void AddController(SDL_JoystickID id, SDL_bool verbose)
     new_controller->axis_state = (AxisState *)SDL_calloc(new_controller->num_axes, sizeof(*new_controller->axis_state));
 
     new_controller->gamepad = SDL_OpenGamepad(id);
-    new_controller->mapping = SDL_GetGamepadMapping(new_controller->gamepad);
-    new_controller->has_bindings = MappingHasBindings(new_controller->mapping);
 
     if (new_controller->gamepad) {
         SDL_Gamepad *gamepad = new_controller->gamepad;
@@ -670,6 +669,9 @@ static void AddController(SDL_JoystickID id, SDL_bool verbose)
     } else {
         SetController(id);
     }
+
+    /* Update the binding state */
+    HandleGamepadRemapped(id);
 }
 
 static void DelController(SDL_JoystickID id)
@@ -714,23 +716,6 @@ static void DelController(SDL_JoystickID id)
     }
 }
 
-static void HandleGamepadRemapped(SDL_JoystickID id)
-{
-    int i = FindController(id);
-
-    if (i < 0) {
-        return;
-    }
-
-    if (!controllers[i].gamepad) {
-        controllers[i].gamepad = SDL_OpenGamepad(id);
-    }
-    if (controllers[i].mapping) {
-        SDL_free(controllers[i].mapping);
-    }
-    controllers[i].mapping = SDL_GetGamepadMapping(controllers[i].gamepad);
-}
-
 static Uint16 ConvertAxisToRumble(Sint16 axisval)
 {
     /* Only start rumbling if the axis is past the halfway point */