Fixed bug 5195 - Replugging in "mixed" controller types crashes on macOS

RustyM

This is related to Bug 5034, but crashes under a somewhat different condition.

In the latest tip (changeset 13914) or with the SDL 2.0.12 source + David?s 5034 patch, unplugging and then replugging in certain controller types on macOS will crash. A mix of new controllers like Switch Pro, PS4 and Xbox One all work without issue. But if a controller without a rumble function, like many SNES retro USB gamepads, is mixed with a PS4 or Switch Pro controller it will crash.

File: joystick/darwin/SDL_sysjoystick.c
Function: static recDevice *FreeDevice(recDevice *removeDevice)
On line 159: while (device->pNext != removeDevice) {
Causes: Thread 1: EXC_BAD_ACCESS (code=1, address=0x188)

This can be reproduced in testgamecontroller" by starting the test program with both a ?retro? controller plugged in and a ?modern rumble? controller (Switch Pro/PS4). This may crash on launch, but it depends on which controller ends up as device 0. If it doesn?t crash, unplug the ?modern rumble? controller and plug it back in.

Some of the "retro" controllers I?ve seen this crash with:
- iBuffalo SNES Controller
- 8Bitdo SN30 Gamepad (in MacOS mode)
- Retrolink NES Controller
- HuiJia SNES Controller Adaptor

The issue appears macOS specific. Seen on 10.12.6 and 10.14.6. Not seen on Windows 10.

The while loop in FreeDevice() assumes that every device is not NULL.

    recDevice *device = gpDeviceList;
    while (device->pNext != removeDevice) {
        device = device->pNext;
    }
    device->pNext = pDeviceNext;

So maybe we should check for NULL here? Or instead prevent adding NULL devices to the list in the first place? Checking device for NULL before entering the loop appears to work.

    recDevice *device = gpDeviceList;
    if (!device) {
        while (device->pNext != removeDevice) {
            device = device->pNext;
        }
    }
    device->pNext = pDeviceNext;

src/joystick/darwin/SDL_iokitjoystick.c

@@ -152,14 +152,17 @@ FreeDevice(recDevice *removeDevice)
         /* save next device prior to disposing of this device */
         pDeviceNext = removeDevice->pNext;
 
-        if ( gpDeviceList == removeDevice ) {
+        if (gpDeviceList == removeDevice) {
             gpDeviceList = pDeviceNext;
         } else if (gpDeviceList) {
-            recDevice *device = gpDeviceList;
-            while (device->pNext != removeDevice) {
-                device = device->pNext;
+            recDevice *device;
+
+            for (device = gpDeviceList; device; device = device->pNext) {
+                if (device->pNext == removeDevice) {
+                    device->pNext = pDeviceNext;
+                    break;
+                }
             }
-            device->pNext = pDeviceNext;
         }
         removeDevice->pNext = NULL;