From e23257307e220c7dd3d827e195f52adaabaaeeeb Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Sat, 1 Jun 2024 22:05:21 -0400 Subject: [PATCH] Introduce formal policy for APIs that return strings. This declares that any `const char *` returned from SDL is owned by SDL, and promises to be valid _at least_ until the next time the event queue runs, or SDL_Quit() is called, even if the thing that owns the string gets destroyed or changed before then. This is noted in the headers as "the SDL_GetStringRule", so this will both be greppable to find a detailed explaination in docs/README-strings.md and wikiheaders will automatically turn it into a link we can point at the appropriate documentation. Fixes #9902. (and several FIXMEs, both known and yet-undocumented.) --- docs/README-strings.md | 58 ++++++++++++++++++++++ include/SDL3/SDL_audio.h | 19 +++---- include/SDL3/SDL_camera.h | 12 +++-- include/SDL3/SDL_error.h | 11 ++-- include/SDL3/SDL_gamepad.h | 20 ++++++-- include/SDL3/SDL_haptic.h | 4 ++ include/SDL3/SDL_hints.h | 2 + include/SDL3/SDL_joystick.h | 10 ++++ include/SDL3/SDL_keyboard.h | 6 +++ include/SDL3/SDL_mouse.h | 2 + include/SDL3/SDL_pen.h | 2 + include/SDL3/SDL_pixels.h | 2 + include/SDL3/SDL_platform.h | 4 +- include/SDL3/SDL_properties.h | 2 + include/SDL3/SDL_render.h | 3 +- include/SDL3/SDL_sensor.h | 4 ++ include/SDL3/SDL_system.h | 8 ++- include/SDL3/SDL_thread.h | 3 +- include/SDL3/SDL_touch.h | 4 +- include/SDL3/SDL_version.h | 2 + include/SDL3/SDL_video.h | 16 ++++++ src/SDL.c | 5 +- src/SDL_hints.c | 10 ++-- src/SDL_internal.h | 7 +++ src/SDL_properties.c | 12 +---- src/audio/SDL_audio.c | 10 ++-- src/camera/SDL_camera.c | 2 + src/core/SDL_core_unsupported.c | 2 +- src/core/android/SDL_android.c | 2 + src/dynapi/SDL_dynapi_procs.h | 2 +- src/events/SDL_events.c | 35 +++++++------ src/events/SDL_keyboard.c | 7 +-- src/events/SDL_mouse.c | 2 +- src/events/SDL_touch.c | 2 +- src/filesystem/winrt/SDL_sysfilesystem.cpp | 1 + src/haptic/SDL_haptic.c | 4 +- src/joystick/SDL_gamepad.c | 23 +++++---- src/joystick/SDL_joystick.c | 13 ++--- src/joystick/apple/SDL_mfijoystick.m | 15 +++--- src/render/SDL_render.c | 1 + src/sensor/SDL_sensor.c | 5 +- src/thread/SDL_thread.c | 8 +-- src/video/SDL_pixels.c | 1 + src/video/SDL_video.c | 2 + test/testaudio.c | 2 +- test/testaudiocapture.c | 3 +- test/testaudiohotplug.c | 3 +- test/testaudioinfo.c | 3 +- test/testautomation_audio.c | 3 +- test/testmultiaudio.c | 3 +- test/testsurround.c | 3 +- 51 files changed, 262 insertions(+), 123 deletions(-) create mode 100644 docs/README-strings.md diff --git a/docs/README-strings.md b/docs/README-strings.md new file mode 100644 index 000000000..f58af02f2 --- /dev/null +++ b/docs/README-strings.md @@ -0,0 +1,58 @@ +# String policies in SDL3. + +## Encoding. + +Unless otherwise specified, all strings in SDL, across all platforms, are +UTF-8 encoded. + + +## The SDL Get String Rule. + +Did you see 'SDL_GetStringRule' in the wiki or headers? Here are the details +that aren't worth copying across dozens of functions' documentation. + +tl;dr: If an SDL function returns a `const char *` string, do not modify or +free it, and if you need to save it, make a copy right away. + +In several cases, SDL wants to return a string to the app, and the question +in any library that does this is: _who owns this thing?_ + +The answer in almost all cases, is that SDL does, but not for long. + +The pointer is only guaranteed to be valid until the next time the event +queue is updated, or SDL_Quit is called. + +The reason for this is memory safety. + +There are several strings that SDL provides that could be freed at +any moment. For example, an app calls SDL_GetAudioDeviceName(), which returns +a string that is part of the internal audio device structure. But, while this +function is returning, the user yanks the USB audio device out of the +computer, and SDL decides to deallocate the structure...and the string! +Now the app is holding a pointer that didn't live long enough to be useful, +and could crash if accessed. + +To avoid this, instead of calling SDL_free on a string as soon as it's done +with it, SDL adds the pointer to a list. This list is freed at specific +points: when the event queue is run (for ongoing cleanup) and when SDL_Quit +is called (to catch things that are just hanging around). This allows the +app to use a string without worrying about it becoming bogus in the middle +of a printf() call. If the app needs it for longer, it should copy it. + +When does "the event queue run"? There are several points: + +- If the app calls SDL_PumpEvents() _from any thread_. +- SDL_PumpEvents is also called by several other APIs internally: + SDL_PollEvent(), SDL_PeepEvents(), SDL_WaitEvent(), + SDL_WaitEventTimeout(), and maybe others. +- If you are using [the main callbacks](README-main-functions), the + event queue can run immediately after any of the callback functions return. + +Note that these are just guaranteed minimum lifespans; any given string +might live much longer--some might even be static memory that is _never_ +deallocated--but this rule promises that the app has a safe window. + +Note that none of this applies if the return value is `char *` instead of +`const char *`. Please see the specific function's documentation for how +to handle those pointers. + diff --git a/include/SDL3/SDL_audio.h b/include/SDL3/SDL_audio.h index 03bddb9b2..97c5a946a 100644 --- a/include/SDL3/SDL_audio.h +++ b/include/SDL3/SDL_audio.h @@ -374,9 +374,11 @@ extern SDL_DECLSPEC int SDLCALL SDL_GetNumAudioDrivers(void); * first (as far as the SDL developers believe) are earlier in the list. * * The names of drivers are all simple, low-ASCII identifiers, like "alsa", - * "coreaudio" or "xaudio2". These never have Unicode characters, and are not + * "coreaudio" or "wasapi". These never have Unicode characters, and are not * meant to be proper names. * + * The returned string follows the SDL_GetStringRule. + * * \param index the index of the audio driver; the value ranges from 0 to * SDL_GetNumAudioDrivers() - 1 * \returns the name of the audio driver at the requested index, or NULL if an @@ -394,11 +396,11 @@ extern SDL_DECLSPEC const char *SDLCALL SDL_GetAudioDriver(int index); /** * Get the name of the current audio driver. * - * The returned string points to internal static memory and thus never becomes - * invalid, even if you quit the audio subsystem and initialize a new driver - * (although such a case would return a different static string from another - * call to this function, of course). As such, you should not modify or free - * the returned string. + * The names of drivers are all simple, low-ASCII identifiers, like "alsa", + * "coreaudio" or "wasapi". These never have Unicode characters, and are not + * meant to be proper names. + * + * The returned string follows the SDL_GetStringRule. * * \returns the name of the current audio driver or NULL if no driver has been * initialized. @@ -470,8 +472,7 @@ extern SDL_DECLSPEC SDL_AudioDeviceID *SDLCALL SDL_GetAudioCaptureDevices(int *c /** * Get the human-readable name of a specific audio device. * - * The string returned by this function is UTF-8 encoded. The caller should - * call SDL_free on the return value when done with it. + * The returned string follows the SDL_GetStringRule. * * \param devid the instance ID of the device to query. * \returns the name of the audio device, or NULL on error. @@ -484,7 +485,7 @@ extern SDL_DECLSPEC SDL_AudioDeviceID *SDLCALL SDL_GetAudioCaptureDevices(int *c * \sa SDL_GetAudioCaptureDevices * \sa SDL_GetDefaultAudioInfo */ -extern SDL_DECLSPEC char *SDLCALL SDL_GetAudioDeviceName(SDL_AudioDeviceID devid); +extern SDL_DECLSPEC const char *SDLCALL SDL_GetAudioDeviceName(SDL_AudioDeviceID devid); /** * Get the current audio format of a specific audio device. diff --git a/include/SDL3/SDL_camera.h b/include/SDL3/SDL_camera.h index a989fd185..c8f2f40c6 100644 --- a/include/SDL3/SDL_camera.h +++ b/include/SDL3/SDL_camera.h @@ -134,6 +134,8 @@ extern SDL_DECLSPEC int SDLCALL SDL_GetNumCameraDrivers(void); * "coremedia" or "android". These never have Unicode characters, and are not * meant to be proper names. * + * The returned string follows the SDL_GetStringRule. + * * \param index the index of the camera driver; the value ranges from 0 to * SDL_GetNumCameraDrivers() - 1 * \returns the name of the camera driver at the requested index, or NULL if @@ -150,11 +152,11 @@ extern SDL_DECLSPEC const char *SDLCALL SDL_GetCameraDriver(int index); /** * Get the name of the current camera driver. * - * The returned string points to internal static memory and thus never becomes - * invalid, even if you quit the camera subsystem and initialize a new driver - * (although such a case would return a different static string from another - * call to this function, of course). As such, you should not modify or free - * the returned string. + * The names of drivers are all simple, low-ASCII identifiers, like "v4l2", + * "coremedia" or "android". These never have Unicode characters, and are not + * meant to be proper names. + * + * The returned string follows the SDL_GetStringRule. * * \returns the name of the current camera driver or NULL if no driver has * been initialized. diff --git a/include/SDL3/SDL_error.h b/include/SDL3/SDL_error.h index 324c4b4b9..b1f62cf96 100644 --- a/include/SDL3/SDL_error.h +++ b/include/SDL3/SDL_error.h @@ -96,15 +96,14 @@ extern SDL_DECLSPEC int SDLCALL SDL_OutOfMemory(void); * Error strings are set per-thread, so an error set in a different thread * will not interfere with the current thread's operation. * - * The returned string is internally allocated and must not be freed by the - * application. + * The returned string does **NOT** follow the SDL_GetStringRule! The + * pointer is valid until the current thread's error string is changed, so + * the caller should make a copy if the string is to be used after calling + * into SDL again. * * \returns a message with information about the specific error that occurred, * or an empty string if there hasn't been an error message set since - * the last call to SDL_ClearError(). The message is only applicable - * when an SDL function has signaled an error. You must check the - * return values of SDL function calls to determine when to - * appropriately call SDL_GetError(). + * the last call to SDL_ClearError(). * * \since This function is available since SDL 3.0.0. * diff --git a/include/SDL3/SDL_gamepad.h b/include/SDL3/SDL_gamepad.h index 8f3c04e67..fb0fd7691 100644 --- a/include/SDL3/SDL_gamepad.h +++ b/include/SDL3/SDL_gamepad.h @@ -500,6 +500,8 @@ extern SDL_DECLSPEC SDL_bool SDLCALL SDL_IsGamepad(SDL_JoystickID instance_id); * * This can be called before any gamepads are opened. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the joystick instance ID * \returns the name of the selected gamepad. If no name can be found, this * function returns NULL; call SDL_GetError() for more information. @@ -516,6 +518,8 @@ extern SDL_DECLSPEC const char *SDLCALL SDL_GetGamepadInstanceName(SDL_JoystickI * * This can be called before any gamepads are opened. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the joystick instance ID * \returns the path of the selected gamepad. If no path can be found, this * function returns NULL; call SDL_GetError() for more information. @@ -748,6 +752,8 @@ extern SDL_DECLSPEC SDL_JoystickID SDLCALL SDL_GetGamepadInstanceID(SDL_Gamepad /** * Get the implementation-dependent name for an opened gamepad. * + * The returned string follows the SDL_GetStringRule. + * * \param gamepad a gamepad identifier previously returned by * SDL_OpenGamepad() * \returns the implementation dependent name for the gamepad, or NULL if @@ -762,6 +768,8 @@ extern SDL_DECLSPEC const char *SDLCALL SDL_GetGamepadName(SDL_Gamepad *gamepad) /** * Get the implementation-dependent path for an opened gamepad. * + * The returned string follows the SDL_GetStringRule. + * * \param gamepad a gamepad identifier previously returned by * SDL_OpenGamepad() * \returns the implementation dependent path for the gamepad, or NULL if @@ -887,6 +895,8 @@ extern SDL_DECLSPEC Uint16 SDLCALL SDL_GetGamepadFirmwareVersion(SDL_Gamepad *ga * * Returns the serial number of the gamepad, or NULL if it is not available. * + * The returned string follows the SDL_GetStringRule. + * * \param gamepad the gamepad object to query. * \returns the serial number, or NULL if unavailable. * @@ -1045,7 +1055,7 @@ extern SDL_DECLSPEC SDL_GamepadType SDLCALL SDL_GetGamepadTypeFromString(const c /** * Convert from an SDL_GamepadType enum to a string. * - * The caller should not SDL_free() the returned string. + * The returned string follows the SDL_GetStringRule. * * \param type an enum value for a given SDL_GamepadType * \returns a string for the given type, or NULL if an invalid type is @@ -1083,7 +1093,7 @@ extern SDL_DECLSPEC SDL_GamepadAxis SDLCALL SDL_GetGamepadAxisFromString(const c /** * Convert from an SDL_GamepadAxis enum to a string. * - * The caller should not SDL_free() the returned string. + * The returned string follows the SDL_GetStringRule. * * \param axis an enum value for a given SDL_GamepadAxis * \returns a string for the given axis, or NULL if an invalid axis is @@ -1158,7 +1168,7 @@ extern SDL_DECLSPEC SDL_GamepadButton SDLCALL SDL_GetGamepadButtonFromString(con /** * Convert from an SDL_GamepadButton enum to a string. * - * The caller should not SDL_free() the returned string. + * The returned string follows the SDL_GetStringRule. * * \param button an enum value for a given SDL_GamepadButton * \returns a string for the given button, or NULL if an invalid button is @@ -1446,6 +1456,8 @@ extern SDL_DECLSPEC void SDLCALL SDL_CloseGamepad(SDL_Gamepad *gamepad); * Return the sfSymbolsName for a given button on a gamepad on Apple * platforms. * + * The returned string follows the SDL_GetStringRule. + * * \param gamepad the gamepad to query * \param button a button on the gamepad * \returns the sfSymbolsName or NULL if the name can't be found @@ -1459,6 +1471,8 @@ extern SDL_DECLSPEC const char* SDLCALL SDL_GetGamepadAppleSFSymbolsNameForButto /** * Return the sfSymbolsName for a given axis on a gamepad on Apple platforms. * + * The returned string follows the SDL_GetStringRule. + * * \param gamepad the gamepad to query * \param axis an axis on the gamepad * \returns the sfSymbolsName or NULL if the name can't be found diff --git a/include/SDL3/SDL_haptic.h b/include/SDL3/SDL_haptic.h index 3a3978356..5c23ab417 100644 --- a/include/SDL3/SDL_haptic.h +++ b/include/SDL3/SDL_haptic.h @@ -948,6 +948,8 @@ extern SDL_DECLSPEC SDL_HapticID *SDLCALL SDL_GetHaptics(int *count); * * This can be called before any haptic devices are opened. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the haptic device instance ID * \returns the name of the selected haptic device. If no name can be found, * this function returns NULL; call SDL_GetError() for more @@ -1011,6 +1013,8 @@ extern SDL_DECLSPEC SDL_HapticID SDLCALL SDL_GetHapticInstanceID(SDL_Haptic *hap /** * Get the implementation dependent name of a haptic device. * + * The returned string follows the SDL_GetStringRule. + * * \param haptic the SDL_Haptic obtained from SDL_OpenJoystick() * \returns the name of the selected haptic device. If no name can be found, * this function returns NULL; call SDL_GetError() for more diff --git a/include/SDL3/SDL_hints.h b/include/SDL3/SDL_hints.h index fb6d41a93..0b6fdbabb 100644 --- a/include/SDL3/SDL_hints.h +++ b/include/SDL3/SDL_hints.h @@ -3806,6 +3806,8 @@ extern SDL_DECLSPEC void SDLCALL SDL_ResetHints(void); /** * Get the value of a hint. * + * The returned string follows the SDL_GetStringRule. + * * \param name the hint to query * \returns the string value of a hint or NULL if the hint isn't set. * diff --git a/include/SDL3/SDL_joystick.h b/include/SDL3/SDL_joystick.h index dd7d8391b..96920d4c0 100644 --- a/include/SDL3/SDL_joystick.h +++ b/include/SDL3/SDL_joystick.h @@ -231,6 +231,8 @@ extern SDL_DECLSPEC SDL_JoystickID *SDLCALL SDL_GetJoysticks(int *count); * * This can be called before any joysticks are opened. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the joystick instance ID * \returns the name of the selected joystick. If no name can be found, this * function returns NULL; call SDL_GetError() for more information. @@ -247,6 +249,8 @@ extern SDL_DECLSPEC const char *SDLCALL SDL_GetJoystickInstanceName(SDL_Joystick * * This can be called before any joysticks are opened. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the joystick instance ID * \returns the path of the selected joystick. If no path can be found, this * function returns NULL; call SDL_GetError() for more information. @@ -669,6 +673,8 @@ extern SDL_DECLSPEC SDL_PropertiesID SDLCALL SDL_GetJoystickProperties(SDL_Joyst /** * Get the implementation dependent name of a joystick. * + * The returned string follows the SDL_GetStringRule. + * * \param joystick the SDL_Joystick obtained from SDL_OpenJoystick() * \returns the name of the selected joystick. If no name can be found, this * function returns NULL; call SDL_GetError() for more information. @@ -682,6 +688,8 @@ extern SDL_DECLSPEC const char *SDLCALL SDL_GetJoystickName(SDL_Joystick *joysti /** * Get the implementation dependent path of a joystick. * + * The returned string follows the SDL_GetStringRule. + * * \param joystick the SDL_Joystick obtained from SDL_OpenJoystick() * \returns the path of the selected joystick. If no path can be found, this * function returns NULL; call SDL_GetError() for more information. @@ -799,6 +807,8 @@ extern SDL_DECLSPEC Uint16 SDLCALL SDL_GetJoystickFirmwareVersion(SDL_Joystick * * * Returns the serial number of the joystick, or NULL if it is not available. * + * The returned string follows the SDL_GetStringRule. + * * \param joystick the SDL_Joystick obtained from SDL_OpenJoystick() * \returns the serial number of the selected joystick, or NULL if * unavailable. diff --git a/include/SDL3/SDL_keyboard.h b/include/SDL3/SDL_keyboard.h index bf2de08aa..791d96ae2 100644 --- a/include/SDL3/SDL_keyboard.h +++ b/include/SDL3/SDL_keyboard.h @@ -109,6 +109,8 @@ extern SDL_DECLSPEC SDL_KeyboardID *SDLCALL SDL_GetKeyboards(int *count); * * This function returns "" if the keyboard doesn't have a name. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the keyboard instance ID * \returns the name of the selected keyboard, or NULL on failure; call * SDL_GetError() for more information. @@ -255,6 +257,8 @@ extern SDL_DECLSPEC SDL_Scancode SDLCALL SDL_GetScancodeFromKey(SDL_Keycode key) * * See SDL_Scancode for details. * + * The returned string follows the SDL_GetStringRule. + * * **Warning**: The returned name is by design not stable across platforms, * e.g. the name for `SDL_SCANCODE_LGUI` is "Left GUI" under Linux but "Left * Windows" under Microsoft Windows, and some scancodes like @@ -295,6 +299,8 @@ extern SDL_DECLSPEC SDL_Scancode SDLCALL SDL_GetScancodeFromName(const char *nam * * See SDL_Scancode and SDL_Keycode for details. * + * The returned string follows the SDL_GetStringRule. + * * \param key the desired SDL_Keycode to query * \returns a pointer to a UTF-8 string that stays valid at least until the * next call to this function. If you need it around any longer, you diff --git a/include/SDL3/SDL_mouse.h b/include/SDL3/SDL_mouse.h index 483f96b83..14ad506a4 100644 --- a/include/SDL3/SDL_mouse.h +++ b/include/SDL3/SDL_mouse.h @@ -152,6 +152,8 @@ extern SDL_DECLSPEC SDL_MouseID *SDLCALL SDL_GetMice(int *count); * * This function returns "" if the mouse doesn't have a name. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the mouse instance ID * \returns the name of the selected mouse, or NULL on failure; call * SDL_GetError() for more information. diff --git a/include/SDL3/SDL_pen.h b/include/SDL3/SDL_pen.h index 41472973b..d341cb31e 100644 --- a/include/SDL3/SDL_pen.h +++ b/include/SDL3/SDL_pen.h @@ -224,6 +224,8 @@ extern SDL_DECLSPEC SDL_bool SDLCALL SDL_PenConnected(SDL_PenID instance_id); /** * Retrieves a human-readable description for a SDL_PenID. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id The pen to query. * \returns A string that contains the name of the pen, intended for human * consumption. The string might or might not be localised, depending diff --git a/include/SDL3/SDL_pixels.h b/include/SDL3/SDL_pixels.h index a969e7966..2b814c0c9 100644 --- a/include/SDL3/SDL_pixels.h +++ b/include/SDL3/SDL_pixels.h @@ -758,6 +758,8 @@ typedef struct SDL_PixelFormat /** * Get the human readable name of a pixel format. * + * The returned string follows the SDL_GetStringRule. + * * \param format the pixel format to query * \returns the human readable name of the specified pixel format or * `SDL_PIXELFORMAT_UNKNOWN` if the format isn't recognized. diff --git a/include/SDL3/SDL_platform.h b/include/SDL3/SDL_platform.h index fa95922e2..b010f43f3 100644 --- a/include/SDL3/SDL_platform.h +++ b/include/SDL3/SDL_platform.h @@ -48,12 +48,14 @@ extern "C" { * - "iOS" * - "Android" * + * The returned string follows the SDL_GetStringRule. + * * \returns the name of the platform. If the correct platform name is not * available, returns a string beginning with the text "Unknown". * * \since This function is available since SDL 3.0.0. */ -extern SDL_DECLSPEC const char * SDLCALL SDL_GetPlatform (void); +extern SDL_DECLSPEC const char * SDLCALL SDL_GetPlatform(void); /* Ends C function definitions when using C++ */ #ifdef __cplusplus diff --git a/include/SDL3/SDL_properties.h b/include/SDL3/SDL_properties.h index 728313878..fc8c4cf52 100644 --- a/include/SDL3/SDL_properties.h +++ b/include/SDL3/SDL_properties.h @@ -360,6 +360,8 @@ extern SDL_DECLSPEC void *SDLCALL SDL_GetProperty(SDL_PropertiesID props, const /** * Get a string property on a set of properties. * + * The returned string follows the SDL_GetStringRule. + * * \param props the properties to query * \param name the name of the property to query * \param default_value the default value of the property diff --git a/include/SDL3/SDL_render.h b/include/SDL3/SDL_render.h index 641e0ddb6..0e23882eb 100644 --- a/include/SDL3/SDL_render.h +++ b/include/SDL3/SDL_render.h @@ -166,8 +166,7 @@ extern SDL_DECLSPEC int SDLCALL SDL_GetNumRenderDrivers(void); * "direct3d12" or "metal". These never have Unicode characters, and are not * meant to be proper names. * - * The returned value points to a static, read-only string; do not modify or - * free it! + * The returned string follows the SDL_GetStringRule. * * \param index the index of the rendering driver; the value ranges from 0 to * SDL_GetNumRenderDrivers() - 1 diff --git a/include/SDL3/SDL_sensor.h b/include/SDL3/SDL_sensor.h index 2adb94b49..94593432c 100644 --- a/include/SDL3/SDL_sensor.h +++ b/include/SDL3/SDL_sensor.h @@ -158,6 +158,8 @@ extern SDL_DECLSPEC SDL_SensorID *SDLCALL SDL_GetSensors(int *count); /** * Get the implementation dependent name of a sensor. * + * The returned string follows the SDL_GetStringRule. + * * \param instance_id the sensor instance ID * \returns the sensor name, or NULL if `instance_id` is not valid * @@ -224,6 +226,8 @@ extern SDL_DECLSPEC SDL_PropertiesID SDLCALL SDL_GetSensorProperties(SDL_Sensor /** * Get the implementation dependent name of a sensor. * + * The returned string follows the SDL_GetStringRule. + * * \param sensor The SDL_Sensor object * \returns the sensor name, or NULL if `sensor` is NULL. * diff --git a/include/SDL3/SDL_system.h b/include/SDL3/SDL_system.h index 9e8b145b2..f5bbb559f 100644 --- a/include/SDL3/SDL_system.h +++ b/include/SDL3/SDL_system.h @@ -385,6 +385,8 @@ extern SDL_DECLSPEC void SDLCALL SDL_AndroidBackButton(void); * Your internal storage path is typically: * `/data/data/your.app.package/files`. * + * The returned string follows the SDL_GetStringRule. + * * \returns the path used for internal storage or NULL on failure; call * SDL_GetError() for more information. * @@ -392,7 +394,7 @@ extern SDL_DECLSPEC void SDLCALL SDL_AndroidBackButton(void); * * \sa SDL_AndroidGetExternalStorageState */ -extern SDL_DECLSPEC const char * SDLCALL SDL_AndroidGetInternalStoragePath(void); +extern SDL_DECLSPEC const char *SDLCALL SDL_AndroidGetInternalStoragePath(void); /** * Get the current state of external storage. @@ -422,6 +424,8 @@ extern SDL_DECLSPEC int SDLCALL SDL_AndroidGetExternalStorageState(Uint32 *state * Your external storage path is typically: * `/storage/sdcard0/Android/data/your.app.package/files`. * + * The returned string follows the SDL_GetStringRule. + * * \returns the path used for external storage for this application on success * or NULL on failure; call SDL_GetError() for more information. * @@ -576,6 +580,8 @@ typedef enum SDL_WinRT_DeviceFamily * * https://msdn.microsoft.com/en-us/library/windows/apps/hh464917.aspx * + * The returned string follows the SDL_GetStringRule. + * * \param pathType the type of path to retrieve, one of SDL_WinRT_Path * \returns a UTF-8 string (8-bit, multi-byte) containing the path, or NULL if * the path is not available for any reason; call SDL_GetError() for diff --git a/include/SDL3/SDL_thread.h b/include/SDL3/SDL_thread.h index 42d590c89..3502b8c15 100644 --- a/include/SDL3/SDL_thread.h +++ b/include/SDL3/SDL_thread.h @@ -331,8 +331,7 @@ extern SDL_DECLSPEC SDL_Thread *SDLCALL SDL_CreateThreadWithPropertiesRuntime(SD /** * Get the thread name as it was specified in SDL_CreateThread(). * - * This is internal memory, not to be freed by the caller, and remains valid - * until the specified thread is cleaned up by SDL_WaitThread(). + * The returned string follows the SDL_GetStringRule. * * \param thread the thread to query * \returns a pointer to a UTF-8 string that names the specified thread, or diff --git a/include/SDL3/SDL_touch.h b/include/SDL3/SDL_touch.h index b8e61a099..df9a7b720 100644 --- a/include/SDL3/SDL_touch.h +++ b/include/SDL3/SDL_touch.h @@ -96,7 +96,7 @@ extern SDL_DECLSPEC SDL_TouchID *SDLCALL SDL_GetTouchDevices(int *count); /** * Get the touch device name as reported from the driver. * - * You do not own the returned string, do not modify or free it. + * The returned string follows the SDL_GetStringRule. * * \param touchID the touch device instance ID. * \returns touch device name, or NULL on error; call SDL_GetError() for more @@ -104,7 +104,7 @@ extern SDL_DECLSPEC SDL_TouchID *SDLCALL SDL_GetTouchDevices(int *count); * * \since This function is available since SDL 3.0.0. */ -extern SDL_DECLSPEC const char* SDLCALL SDL_GetTouchDeviceName(SDL_TouchID touchID); +extern SDL_DECLSPEC const char *SDLCALL SDL_GetTouchDeviceName(SDL_TouchID touchID); /** * Get the type of the given touch device. diff --git a/include/SDL3/SDL_version.h b/include/SDL3/SDL_version.h index 7852b565a..d6518ca5a 100644 --- a/include/SDL3/SDL_version.h +++ b/include/SDL3/SDL_version.h @@ -143,6 +143,8 @@ extern SDL_DECLSPEC int SDLCALL SDL_GetVersion(void); * You shouldn't use this function for anything but logging it for debugging * purposes. The string is not intended to be reliable in any way. * + * The returned string follows the SDL_GetStringRule. + * * \returns an arbitrary string, uniquely identifying the exact revision of * the SDL library in use. * diff --git a/include/SDL3/SDL_video.h b/include/SDL3/SDL_video.h index 97cbab5a7..7622d0764 100644 --- a/include/SDL3/SDL_video.h +++ b/include/SDL3/SDL_video.h @@ -338,6 +338,12 @@ extern SDL_DECLSPEC int SDLCALL SDL_GetNumVideoDrivers(void); * The video drivers are presented in the order in which they are normally * checked during initialization. * + * The names of drivers are all simple, low-ASCII identifiers, like "cocoa", + * "x11" or "windows". These never have Unicode characters, and are not + * meant to be proper names. + * + * The returned string follows the SDL_GetStringRule. + * * \param index the index of a video driver * \returns the name of the video driver with the given **index**. * @@ -350,6 +356,12 @@ extern SDL_DECLSPEC const char *SDLCALL SDL_GetVideoDriver(int index); /** * Get the name of the currently initialized video driver. * + * The names of drivers are all simple, low-ASCII identifiers, like "cocoa", + * "x11" or "windows". These never have Unicode characters, and are not + * meant to be proper names. + * + * The returned string follows the SDL_GetStringRule. + * * \returns the name of the current video driver or NULL if no driver has been * initialized. * @@ -438,6 +450,8 @@ extern SDL_DECLSPEC SDL_PropertiesID SDLCALL SDL_GetDisplayProperties(SDL_Displa /** * Get the name of a display in UTF-8 encoding. * + * The returned string follows the SDL_GetStringRule. + * * \param displayID the instance ID of the display to query * \returns the name of a display or NULL on failure; call SDL_GetError() for * more information. @@ -1260,6 +1274,8 @@ extern SDL_DECLSPEC int SDLCALL SDL_SetWindowTitle(SDL_Window *window, const cha /** * Get the title of a window. * + * The returned string follows the SDL_GetStringRule. + * * \param window the window to query * \returns the title of the window in UTF-8 format or "" if there is no * title. diff --git a/src/SDL.c b/src/SDL.c index 86c75f05b..a784810d5 100644 --- a/src/SDL.c +++ b/src/SDL.c @@ -573,10 +573,11 @@ int SDL_GetVersion(void) /* Get the library source revision */ const char *SDL_GetRevision(void) { - return SDL_REVISION; + return SDL_REVISION; // a string literal, no need to SDL_FreeLater it. } -/* Get the name of the platform */ +// Get the name of the platform +// (a string literal, no need to SDL_FreeLater it.) const char *SDL_GetPlatform(void) { #if defined(SDL_PLATFORM_AIX) diff --git a/src/SDL_hints.c b/src/SDL_hints.c index 95d57f134..905264523 100644 --- a/src/SDL_hints.c +++ b/src/SDL_hints.c @@ -74,9 +74,7 @@ SDL_bool SDL_SetHintWithPriority(const char *name, const char *value, SDL_HintPr entry->callback(entry->userdata, name, old_value, value); entry = next; } - if (old_value) { - SDL_free(old_value); - } + SDL_FreeLater(old_value); } hint->priority = priority; return SDL_TRUE; @@ -120,7 +118,7 @@ SDL_bool SDL_ResetHint(const char *name) entry = next; } } - SDL_free(hint->value); + SDL_FreeLater(hint->value); hint->value = NULL; hint->priority = SDL_HINT_DEFAULT; return SDL_TRUE; @@ -147,7 +145,7 @@ void SDL_ResetHints(void) entry = next; } } - SDL_free(hint->value); + SDL_FreeLater(hint->value); hint->value = NULL; hint->priority = SDL_HINT_DEFAULT; } @@ -305,7 +303,7 @@ void SDL_ClearHints(void) SDL_hints = hint->next; SDL_free(hint->name); - SDL_free(hint->value); + SDL_FreeLater(hint->value); for (entry = hint->callbacks; entry;) { SDL_HintWatch *freeable = entry; entry = entry->next; diff --git a/src/SDL_internal.h b/src/SDL_internal.h index a1bdbdec2..6a1a3a313 100644 --- a/src/SDL_internal.h +++ b/src/SDL_internal.h @@ -293,6 +293,13 @@ extern int SDLCALL SDL_WaitSemaphoreTimeoutNS(SDL_Semaphore *sem, Sint64 timeout extern int SDLCALL SDL_WaitConditionTimeoutNS(SDL_Condition *cond, SDL_Mutex *mutex, Sint64 timeoutNS); extern SDL_bool SDLCALL SDL_WaitEventTimeoutNS(SDL_Event *event, Sint64 timeoutNS); +/* Queue `memory` to be passed to SDL_free once the event queue is emptied. + this manages the list of pointers to SDL_AllocateEventMemory, but you + can use it to queue pointers from other subsystems that can die at any + moment but definitely need to live long enough for the app to copy them + if they happened to query them in their last moments. */ +extern void *SDL_FreeLater(void *memory); + /* Ends C function definitions when using C++ */ #ifdef __cplusplus } diff --git a/src/SDL_properties.c b/src/SDL_properties.c index 663414558..0b584213e 100644 --- a/src/SDL_properties.c +++ b/src/SDL_properties.c @@ -65,14 +65,12 @@ static void SDL_FreePropertyWithCleanup(const void *key, const void *value, void } break; case SDL_PROPERTY_TYPE_STRING: - SDL_free(property->value.string_value); + SDL_FreeLater(property->value.string_value); // this pointer might be given to the app by SDL_GetStringProperty. break; default: break; } - if (property->string_storage) { - SDL_free(property->string_storage); - } + SDL_FreeLater(property->string_storage); // this pointer might be given to the app by SDL_GetStringProperty. } SDL_free((void *)key); SDL_free((void *)value); @@ -552,12 +550,6 @@ const char *SDL_GetStringProperty(SDL_PropertiesID props, const char *name, cons return value; } - /* Note that taking the lock here only guarantees that we won't read the - * hashtable while it's being modified. The value itself can easily be - * freed from another thread after it is returned here. - * - * FIXME: Should we SDL_strdup() the return value to avoid this? - */ SDL_LockMutex(properties->lock); { SDL_Property *property = NULL; diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c index 521333ec0..8f86ba414 100644 --- a/src/audio/SDL_audio.c +++ b/src/audio/SDL_audio.c @@ -131,6 +131,7 @@ int SDL_GetNumAudioDrivers(void) return num_drivers; } +// this returns string literals, so there's no need to use SDL_FreeLater. const char *SDL_GetAudioDriver(int index) { if (index >= 0 && index < SDL_GetNumAudioDrivers()) { @@ -139,6 +140,7 @@ const char *SDL_GetAudioDriver(int index) return NULL; } +// this returns string literals, so there's no need to use SDL_FreeLater. const char *SDL_GetCurrentAudioDriver(void) { return current_audio.name; @@ -521,7 +523,7 @@ static void DestroyPhysicalAudioDevice(SDL_AudioDevice *device) SDL_DestroyMutex(device->lock); SDL_DestroyCondition(device->close_cond); SDL_free(device->work_buffer); - SDL_free(device->name); + SDL_FreeLater(device->name); // this pointer is handed to the app during SDL_GetAudioDeviceName SDL_free(device); } @@ -1402,12 +1404,12 @@ SDL_AudioDevice *SDL_FindPhysicalAudioDeviceByHandle(void *handle) return SDL_FindPhysicalAudioDeviceByCallback(TestDeviceHandleCallback, handle); } -char *SDL_GetAudioDeviceName(SDL_AudioDeviceID devid) +const char *SDL_GetAudioDeviceName(SDL_AudioDeviceID devid) { - char *retval = NULL; + const char *retval = NULL; SDL_AudioDevice *device = ObtainPhysicalAudioDevice(devid); if (device) { - retval = SDL_strdup(device->name); + retval = device->name; } ReleaseAudioDevice(device); diff --git a/src/camera/SDL_camera.c b/src/camera/SDL_camera.c index 3b337fb89..5b567be4e 100644 --- a/src/camera/SDL_camera.c +++ b/src/camera/SDL_camera.c @@ -63,6 +63,7 @@ int SDL_GetNumCameraDrivers(void) return SDL_arraysize(bootstrap) - 1; } +// this returns string literals, so there's no need to use SDL_FreeLater. const char *SDL_GetCameraDriver(int index) { if (index >= 0 && index < SDL_GetNumCameraDrivers()) { @@ -71,6 +72,7 @@ const char *SDL_GetCameraDriver(int index) return NULL; } +// this returns string literals, so there's no need to use SDL_FreeLater. const char *SDL_GetCurrentCameraDriver(void) { return camera_driver.name; diff --git a/src/core/SDL_core_unsupported.c b/src/core/SDL_core_unsupported.c index 71eb25d45..eb1370795 100644 --- a/src/core/SDL_core_unsupported.c +++ b/src/core/SDL_core_unsupported.c @@ -141,7 +141,7 @@ int SDL_AndroidGetExternalStorageState(Uint32 *state) return SDL_Unsupported(); } SDL_DECLSPEC const char *SDLCALL SDL_AndroidGetInternalStoragePath(void); -const char *SDL_AndroidGetInternalStoragePath() +const char *SDL_AndroidGetInternalStoragePath(void) { SDL_Unsupported(); return NULL; diff --git a/src/core/android/SDL_android.c b/src/core/android/SDL_android.c index 48e553db5..cce8e7a53 100644 --- a/src/core/android/SDL_android.c +++ b/src/core/android/SDL_android.c @@ -2417,6 +2417,7 @@ void SDL_AndroidBackButton(void) (*env)->CallStaticVoidMethod(env, mActivityClass, midManualBackButton); } +// this caches a string until the process ends, so there's no need to use SDL_FreeLater. const char *SDL_AndroidGetInternalStoragePath(void) { static char *s_AndroidInternalFilesPath = NULL; @@ -2516,6 +2517,7 @@ int SDL_AndroidGetExternalStorageState(Uint32 *state) return 0; } +// this caches a string until the process ends, so there's no need to use SDL_FreeLater. const char *SDL_AndroidGetExternalStoragePath(void) { static char *s_AndroidExternalFilesPath = NULL; diff --git a/src/dynapi/SDL_dynapi_procs.h b/src/dynapi/SDL_dynapi_procs.h index e77bea098..43983a511 100644 --- a/src/dynapi/SDL_dynapi_procs.h +++ b/src/dynapi/SDL_dynapi_procs.h @@ -213,7 +213,7 @@ SDL_DYNAPI_PROC(SDL_AssertionHandler,SDL_GetAssertionHandler,(void **a),(a),retu SDL_DYNAPI_PROC(const SDL_AssertData*,SDL_GetAssertionReport,(void),(),return) SDL_DYNAPI_PROC(SDL_AudioDeviceID*,SDL_GetAudioCaptureDevices,(int *a),(a),return) SDL_DYNAPI_PROC(int,SDL_GetAudioDeviceFormat,(SDL_AudioDeviceID a, SDL_AudioSpec *b, int *c),(a,b,c),return) -SDL_DYNAPI_PROC(char*,SDL_GetAudioDeviceName,(SDL_AudioDeviceID a),(a),return) +SDL_DYNAPI_PROC(const char*,SDL_GetAudioDeviceName,(SDL_AudioDeviceID a),(a),return) SDL_DYNAPI_PROC(const char*,SDL_GetAudioDriver,(int a),(a),return) SDL_DYNAPI_PROC(SDL_AudioDeviceID*,SDL_GetAudioOutputDevices,(int *a),(a),return) SDL_DYNAPI_PROC(int,SDL_GetAudioStreamAvailable,(SDL_AudioStream *a),(a),return) diff --git a/src/events/SDL_events.c b/src/events/SDL_events.c index 61a4da61d..f542ce160 100644 --- a/src/events/SDL_events.c +++ b/src/events/SDL_events.c @@ -108,37 +108,40 @@ static SDL_Mutex *SDL_event_memory_lock; static SDL_EventMemory *SDL_event_memory_head; static SDL_EventMemory *SDL_event_memory_tail; -void *SDL_AllocateEventMemory(size_t size) +void *SDL_FreeLater(void *memory) { - void *memory = SDL_malloc(size); - if (!memory) { + if (memory == NULL) { return NULL; } + SDL_EventMemory *entry = (SDL_EventMemory *)SDL_malloc(sizeof(*entry)); + if (!entry) { + return memory; // this is now a leak, but you probably have bigger problems if malloc failed. We could probably pool up and reuse entries, though. + } + SDL_LockMutex(SDL_event_memory_lock); { - SDL_EventMemory *entry = (SDL_EventMemory *)SDL_malloc(sizeof(*entry)); - if (entry) { - entry->eventID = SDL_last_event_id; - entry->memory = memory; - entry->next = NULL; + entry->eventID = SDL_last_event_id; + entry->memory = memory; + entry->next = NULL; - if (SDL_event_memory_tail) { - SDL_event_memory_tail->next = entry; - } else { - SDL_event_memory_head = entry; - } - SDL_event_memory_tail = entry; + if (SDL_event_memory_tail) { + SDL_event_memory_tail->next = entry; } else { - SDL_free(memory); - memory = NULL; + SDL_event_memory_head = entry; } + SDL_event_memory_tail = entry; } SDL_UnlockMutex(SDL_event_memory_lock); return memory; } +void *SDL_AllocateEventMemory(size_t size) +{ + return SDL_FreeLater(SDL_malloc(size)); +} + static void SDL_FlushEventMemory(Uint32 eventID) { SDL_LockMutex(SDL_event_memory_lock); diff --git a/src/events/SDL_keyboard.c b/src/events/SDL_keyboard.c index b6336582a..16571952a 100644 --- a/src/events/SDL_keyboard.c +++ b/src/events/SDL_keyboard.c @@ -742,7 +742,7 @@ void SDL_RemoveKeyboard(SDL_KeyboardID keyboardID, SDL_bool send_event) return; } - SDL_free(SDL_keyboards[keyboard_index].name); + SDL_FreeLater(SDL_keyboards[keyboard_index].name); if (keyboard_index != SDL_keyboard_count - 1) { SDL_memcpy(&SDL_keyboards[keyboard_index], &SDL_keyboards[keyboard_index + 1], (SDL_keyboard_count - keyboard_index - 1) * sizeof(SDL_keyboards[keyboard_index])); @@ -1334,6 +1334,7 @@ SDL_Scancode SDL_GetScancodeFromKey(SDL_Keycode key) return SDL_SCANCODE_UNKNOWN; } +// these are static memory, so we don't use SDL_FreeLater on them. const char *SDL_GetScancodeName(SDL_Scancode scancode) { const char *name; @@ -1374,7 +1375,7 @@ SDL_Scancode SDL_GetScancodeFromName(const char *name) const char *SDL_GetKeyName(SDL_Keycode key) { - static char name[8]; + char name[8]; char *end; if (key & SDLK_SCANCODE_MASK) { @@ -1405,7 +1406,7 @@ const char *SDL_GetKeyName(SDL_Keycode key) end = SDL_UCS4ToUTF8((Uint32)key, name); *end = '\0'; - return name; + return SDL_FreeLater(SDL_strdup(name)); } } diff --git a/src/events/SDL_mouse.c b/src/events/SDL_mouse.c index 3dd61a123..f550ab9cb 100644 --- a/src/events/SDL_mouse.c +++ b/src/events/SDL_mouse.c @@ -288,7 +288,7 @@ void SDL_RemoveMouse(SDL_MouseID mouseID, SDL_bool send_event) return; } - SDL_free(SDL_mice[mouse_index].name); + SDL_FreeLater(SDL_mice[mouse_index].name); // SDL_GetMouseInstanceName returns this pointer. if (mouse_index != SDL_mouse_count - 1) { SDL_memcpy(&SDL_mice[mouse_index], &SDL_mice[mouse_index + 1], (SDL_mouse_count - mouse_index - 1) * sizeof(SDL_mice[mouse_index])); diff --git a/src/events/SDL_touch.c b/src/events/SDL_touch.c index e47154d9b..160289bb2 100644 --- a/src/events/SDL_touch.c +++ b/src/events/SDL_touch.c @@ -493,7 +493,7 @@ void SDL_DelTouch(SDL_TouchID id) SDL_free(touch->fingers[i]); } SDL_free(touch->fingers); - SDL_free(touch->name); + SDL_FreeLater(touch->name); // this pointer might be given to the app by SDL_GetTouchDeviceName. SDL_free(touch); SDL_num_touch--; diff --git a/src/filesystem/winrt/SDL_sysfilesystem.cpp b/src/filesystem/winrt/SDL_sysfilesystem.cpp index f29bc348a..f253ecb67 100644 --- a/src/filesystem/winrt/SDL_sysfilesystem.cpp +++ b/src/filesystem/winrt/SDL_sysfilesystem.cpp @@ -93,6 +93,7 @@ static const wchar_t *SDL_WinRTGetFSPathUNICODE(SDL_WinRT_Path pathType) return NULL; } +// this caches a string until the process ends, so there's no need to use SDL_FreeLater. extern "C" const char *SDL_WinRTGetFSPath(SDL_WinRT_Path pathType) { typedef unordered_map UTF8PathMap; diff --git a/src/haptic/SDL_haptic.c b/src/haptic/SDL_haptic.c index 795cff1af..9d852b3dd 100644 --- a/src/haptic/SDL_haptic.c +++ b/src/haptic/SDL_haptic.c @@ -100,7 +100,7 @@ const char *SDL_GetHapticInstanceName(SDL_HapticID instance_id) if (SDL_GetHapticIndex(instance_id, &device_index)) { name = SDL_SYS_HapticName(device_index); } - return name; + return name ? SDL_FreeLater(SDL_strdup(name)) : NULL; } SDL_Haptic *SDL_OpenHaptic(SDL_HapticID instance_id) @@ -338,7 +338,7 @@ void SDL_CloseHaptic(SDL_Haptic *haptic) } /* Free the data associated with this device */ - SDL_free(haptic->name); + SDL_FreeLater(haptic->name); // this pointer is handed to the app in SDL_GetHapticName() SDL_free(haptic); } diff --git a/src/joystick/SDL_gamepad.c b/src/joystick/SDL_gamepad.c index 50b122f75..854afa3ac 100644 --- a/src/joystick/SDL_gamepad.c +++ b/src/joystick/SDL_gamepad.c @@ -1594,7 +1594,7 @@ static GamepadMapping_t *SDL_PrivateAddMappingForGUID(SDL_JoystickGUID jGUID, co /* Only overwrite the mapping if the priority is the same or higher. */ if (pGamepadMapping->priority <= priority) { /* Update existing mapping */ - SDL_free(pGamepadMapping->name); + SDL_FreeLater(pGamepadMapping->name); // this is returned in SDL_GetGamepadName. pGamepadMapping->name = pchName; SDL_free(pGamepadMapping->mapping); pGamepadMapping->mapping = pchMapping; @@ -3413,7 +3413,8 @@ const char * SDL_GetGamepadSerial(SDL_Gamepad *gamepad) if (!joystick) { return NULL; } - return SDL_GetJoystickSerial(joystick); + return SDL_GetJoystickSerial(joystick); // this already returns a SDL_FreeLater pointer. + } Uint64 SDL_GetGamepadSteamHandle(SDL_Gamepad *gamepad) @@ -3680,7 +3681,7 @@ void SDL_QuitGamepadMappings(void) while (s_pSupportedGamepads) { pGamepadMap = s_pSupportedGamepads; s_pSupportedGamepads = s_pSupportedGamepads->next; - SDL_free(pGamepadMap->name); + SDL_FreeLater(pGamepadMap->name); // this is returned in SDL_GetGamepadName. SDL_free(pGamepadMap->mapping); SDL_free(pGamepadMap); } @@ -3826,8 +3827,8 @@ void SDL_GamepadHandleDelayedGuideButton(SDL_Joystick *joystick) const char *SDL_GetGamepadAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_GamepadButton button) { #ifdef SDL_JOYSTICK_MFI - const char *IOS_GetAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_GamepadButton button); - const char *retval; + char *IOS_GetAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_GamepadButton button); + char *retval; SDL_LockJoysticks(); { @@ -3837,9 +3838,11 @@ const char *SDL_GetGamepadAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_ } SDL_UnlockJoysticks(); + // retval was malloc'd by IOS_GetAppleSFSymbolsNameForButton if (retval && *retval) { - return retval; + return SDL_FreeLater(retval); } + SDL_free(retval); #endif return NULL; } @@ -3847,8 +3850,8 @@ const char *SDL_GetGamepadAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_ const char *SDL_GetGamepadAppleSFSymbolsNameForAxis(SDL_Gamepad *gamepad, SDL_GamepadAxis axis) { #ifdef SDL_JOYSTICK_MFI - const char *IOS_GetAppleSFSymbolsNameForAxis(SDL_Gamepad *gamepad, SDL_GamepadAxis axis); - const char *retval; + char *IOS_GetAppleSFSymbolsNameForAxis(SDL_Gamepad *gamepad, SDL_GamepadAxis axis); + char *retval; SDL_LockJoysticks(); { @@ -3858,9 +3861,11 @@ const char *SDL_GetGamepadAppleSFSymbolsNameForAxis(SDL_Gamepad *gamepad, SDL_Ga } SDL_UnlockJoysticks(); + // retval was malloc'd by IOS_GetAppleSFSymbolsNameForAxis if (retval && *retval) { - return retval; + return SDL_FreeLater(retval); } + SDL_free(retval); #endif return NULL; } diff --git a/src/joystick/SDL_joystick.c b/src/joystick/SDL_joystick.c index 001180453..271871e6e 100644 --- a/src/joystick/SDL_joystick.c +++ b/src/joystick/SDL_joystick.c @@ -785,8 +785,7 @@ const char *SDL_GetJoystickInstanceName(SDL_JoystickID instance_id) } SDL_UnlockJoysticks(); - /* FIXME: Really we should reference count this name so it doesn't go away after unlock */ - return name; + return name ? SDL_FreeLater(SDL_strdup(name)) : NULL; } /* @@ -804,11 +803,10 @@ const char *SDL_GetJoystickInstancePath(SDL_JoystickID instance_id) } SDL_UnlockJoysticks(); - /* FIXME: Really we should reference count this path so it doesn't go away after unlock */ if (!path) { SDL_Unsupported(); } - return path; + return path ? SDL_FreeLater(SDL_strdup(path)) : NULL; } /* @@ -1663,7 +1661,6 @@ const char *SDL_GetJoystickName(SDL_Joystick *joystick) } SDL_UnlockJoysticks(); - /* FIXME: Really we should reference count this name so it doesn't go away after unlock */ return retval; } @@ -1888,9 +1885,9 @@ void SDL_CloseJoystick(SDL_Joystick *joystick) } /* Free the data associated with this joystick */ - SDL_free(joystick->name); - SDL_free(joystick->path); - SDL_free(joystick->serial); + SDL_FreeLater(joystick->name); // SDL_GetJoystickName returns this pointer. + SDL_FreeLater(joystick->path); // SDL_GetJoystickPath returns this pointer. + SDL_FreeLater(joystick->serial); // SDL_GetJoystickSerial returns this pointer. SDL_free(joystick->axes); SDL_free(joystick->balls); SDL_free(joystick->hats); diff --git a/src/joystick/apple/SDL_mfijoystick.m b/src/joystick/apple/SDL_mfijoystick.m index a0c677b5c..97a3a83f0 100644 --- a/src/joystick/apple/SDL_mfijoystick.m +++ b/src/joystick/apple/SDL_mfijoystick.m @@ -1916,11 +1916,11 @@ static GCControllerDirectionPad *GetDirectionalPadForController(GCController *co } #endif /* SDL_JOYSTICK_MFI && ENABLE_PHYSICAL_INPUT_PROFILE */ -static char elementName[256]; - -const char *IOS_GetAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_GamepadButton button) +char *IOS_GetAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_GamepadButton button) { + char elementName[256]; elementName[0] = '\0'; + #if defined(SDL_JOYSTICK_MFI) && defined(ENABLE_PHYSICAL_INPUT_PROFILE) if (gamepad && SDL_GetGamepadJoystick(gamepad)->driver == &SDL_IOS_JoystickDriver) { if (@available(macOS 10.16, iOS 14.0, tvOS 14.0, *)) { @@ -2030,12 +2030,15 @@ const char *IOS_GetAppleSFSymbolsNameForButton(SDL_Gamepad *gamepad, SDL_Gamepad } } #endif - return elementName; + + return SDL_strdup(elementName); } -const char *IOS_GetAppleSFSymbolsNameForAxis(SDL_Gamepad *gamepad, SDL_GamepadAxis axis) +char *IOS_GetAppleSFSymbolsNameForAxis(SDL_Gamepad *gamepad, SDL_GamepadAxis axis) { + char elementName[256]; elementName[0] = '\0'; + #if defined(SDL_JOYSTICK_MFI) && defined(ENABLE_PHYSICAL_INPUT_PROFILE) if (gamepad && SDL_GetGamepadJoystick(gamepad)->driver == &SDL_IOS_JoystickDriver) { if (@available(macOS 10.16, iOS 14.0, tvOS 14.0, *)) { @@ -2068,7 +2071,7 @@ const char *IOS_GetAppleSFSymbolsNameForAxis(SDL_Gamepad *gamepad, SDL_GamepadAx } } #endif - return *elementName ? elementName : NULL; + return *elementName ? SDL_strdup(elementName) : NULL; } SDL_JoystickDriver SDL_IOS_JoystickDriver = { diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c index a32bcf366..98c436b69 100644 --- a/src/render/SDL_render.c +++ b/src/render/SDL_render.c @@ -780,6 +780,7 @@ int SDL_GetNumRenderDrivers(void) #endif } +// this returns string literals, so there's no need to use SDL_FreeLater. const char *SDL_GetRenderDriver(int index) { #ifndef SDL_RENDER_DISABLED diff --git a/src/sensor/SDL_sensor.c b/src/sensor/SDL_sensor.c index 969eb6d87..e584788f1 100644 --- a/src/sensor/SDL_sensor.c +++ b/src/sensor/SDL_sensor.c @@ -250,8 +250,7 @@ const char *SDL_GetSensorInstanceName(SDL_SensorID instance_id) } SDL_UnlockSensors(); - /* FIXME: Really we should reference count this name so it doesn't go away after unlock */ - return name; + return name ? SDL_FreeLater(SDL_strdup(name)) : NULL; } SDL_SensorType SDL_GetSensorInstanceType(SDL_SensorID instance_id) @@ -527,7 +526,7 @@ void SDL_CloseSensor(SDL_Sensor *sensor) } /* Free the data associated with this sensor */ - SDL_free(sensor->name); + SDL_FreeLater(sensor->name); // this pointer gets handed to the app by SDL_GetSensorName(). SDL_free(sensor); } SDL_UnlockSensors(); diff --git a/src/thread/SDL_thread.c b/src/thread/SDL_thread.c index c7c82b1f3..909dc7829 100644 --- a/src/thread/SDL_thread.c +++ b/src/thread/SDL_thread.c @@ -299,9 +299,7 @@ void SDL_RunThread(SDL_Thread *thread) if (!SDL_AtomicCompareAndSwap(&thread->state, SDL_THREAD_STATE_ALIVE, SDL_THREAD_STATE_ZOMBIE)) { /* Clean up if something already detached us. */ if (SDL_AtomicCompareAndSwap(&thread->state, SDL_THREAD_STATE_DETACHED, SDL_THREAD_STATE_CLEANED)) { - if (thread->name) { - SDL_free(thread->name); - } + SDL_FreeLater(thread->name); SDL_free(thread); } } @@ -421,9 +419,7 @@ void SDL_WaitThread(SDL_Thread *thread, int *status) if (status) { *status = thread->status; } - if (thread->name) { - SDL_free(thread->name); - } + SDL_FreeLater(thread->name); SDL_free(thread); } } diff --git a/src/video/SDL_pixels.c b/src/video/SDL_pixels.c index 980ba6e4a..c87d92d18 100644 --- a/src/video/SDL_pixels.c +++ b/src/video/SDL_pixels.c @@ -85,6 +85,7 @@ Uint16 SDL_expand_byte10[] = { /* Helper functions */ +// This doesn't need SDL_FreeLater since it returns string literals. #define CASE(X) \ case X: \ return #X; diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c index 4a25e972e..f8eb2a8e6 100644 --- a/src/video/SDL_video.c +++ b/src/video/SDL_video.c @@ -498,6 +498,7 @@ int SDL_GetNumVideoDrivers(void) return SDL_arraysize(bootstrap) - 1; } +// this returns string literals, so there's no need to use SDL_FreeLater. const char *SDL_GetVideoDriver(int index) { if (index >= 0 && index < SDL_GetNumVideoDrivers()) { @@ -642,6 +643,7 @@ pre_driver_error: return -1; } +// this returns string literals, so there's no need to use SDL_FreeLater. const char *SDL_GetCurrentVideoDriver(void) { if (!_this) { diff --git a/test/testaudio.c b/test/testaudio.c index 17abda448..ea22dbb3d 100644 --- a/test/testaudio.c +++ b/test/testaudio.c @@ -973,7 +973,7 @@ static Thing *CreatePhysicalDeviceThing(const SDL_AudioDeviceID which, const SDL if (thing) { thing->data.physdev.devid = which; thing->data.physdev.iscapture = iscapture; - thing->data.physdev.name = SDL_GetAudioDeviceName(which); + thing->data.physdev.name = SDL_strdup(SDL_GetAudioDeviceName(which)); thing->ondrag = DeviceThing_ondrag; thing->ondrop = PhysicalDeviceThing_ondrop; thing->ontick = PhysicalDeviceThing_ontick; diff --git a/test/testaudiocapture.c b/test/testaudiocapture.c index 964f022e8..738b0a583 100644 --- a/test/testaudiocapture.c +++ b/test/testaudiocapture.c @@ -81,12 +81,11 @@ int SDL_AppInit(void **appstate, int argc, char **argv) devices = SDL_GetAudioCaptureDevices(NULL); for (i = 0; devices[i] != 0; i++) { - char *name = SDL_GetAudioDeviceName(devices[i]); + const char *name = SDL_GetAudioDeviceName(devices[i]); SDL_Log(" Capture device #%d: '%s'\n", i, name); if (devname && (SDL_strcmp(devname, name) == 0)) { want_device = devices[i]; } - SDL_free(name); } if (devname && (want_device == SDL_AUDIO_DEVICE_DEFAULT_CAPTURE)) { diff --git a/test/testaudiohotplug.c b/test/testaudiohotplug.c index 12b3a4510..f5d0aa994 100644 --- a/test/testaudiohotplug.c +++ b/test/testaudiohotplug.c @@ -71,7 +71,7 @@ static void iteration(void) } else if (e.type == SDL_EVENT_AUDIO_DEVICE_ADDED) { const SDL_AudioDeviceID which = (SDL_AudioDeviceID) e.adevice.which; const SDL_bool iscapture = e.adevice.iscapture ? SDL_TRUE : SDL_FALSE; - char *name = SDL_GetAudioDeviceName(which); + const char *name = SDL_GetAudioDeviceName(which); if (name) { SDL_Log("New %s audio device at id %u: %s", devtypestr(iscapture), (unsigned int)which, name); } else { @@ -92,7 +92,6 @@ static void iteration(void) /* !!! FIXME: this is leaking the stream for now. We'll wire it up to a dictionary or whatever later. */ } } - SDL_free(name); } else if (e.type == SDL_EVENT_AUDIO_DEVICE_REMOVED) { dev = (SDL_AudioDeviceID)e.adevice.which; SDL_Log("%s device %u removed.\n", devtypestr(e.adevice.iscapture), (unsigned int)dev); diff --git a/test/testaudioinfo.c b/test/testaudioinfo.c index 270e10603..2f5243d1e 100644 --- a/test/testaudioinfo.c +++ b/test/testaudioinfo.c @@ -30,10 +30,9 @@ print_devices(SDL_bool iscapture) int i; SDL_Log("Found %d %s device%s:\n", n, typestr, n != 1 ? "s" : ""); for (i = 0; i < n; i++) { - char *name = SDL_GetAudioDeviceName(devices[i]); + const char *name = SDL_GetAudioDeviceName(devices[i]); if (name) { SDL_Log(" %d: %s\n", i, name); - SDL_free(name); } else { SDL_Log(" %d Error: %s\n", i, SDL_GetError()); } diff --git a/test/testautomation_audio.c b/test/testautomation_audio.c index 4747fbb88..08f112cec 100644 --- a/test/testautomation_audio.c +++ b/test/testautomation_audio.c @@ -365,7 +365,7 @@ static int audio_enumerateAndNameAudioDevices(void *arg) { int t; int i, n; - char *name; + const char *name; SDL_AudioDeviceID *devices = NULL; /* Iterate over types: t=0 output device, t=1 input/capture device */ @@ -385,7 +385,6 @@ static int audio_enumerateAndNameAudioDevices(void *arg) SDLTest_AssertCheck(name != NULL, "Verify result from SDL_GetAudioDeviceName(%i) is not NULL", i); if (name != NULL) { SDLTest_AssertCheck(name[0] != '\0', "verify result from SDL_GetAudioDeviceName(%i) is not empty, got: '%s'", i, name); - SDL_free(name); } } } diff --git a/test/testmultiaudio.c b/test/testmultiaudio.c index 7c2755dfc..791a019fb 100644 --- a/test/testmultiaudio.c +++ b/test/testmultiaudio.c @@ -57,7 +57,7 @@ test_multi_audio(SDL_AudioDeviceID *devices, int devcount) #endif for (i = 0; i < devcount; i++) { - char *devname = SDL_GetAudioDeviceName(devices[i]); + const char *devname = SDL_GetAudioDeviceName(devices[i]); SDL_Log("Playing on device #%d of %d: id=%u, name='%s'...", i, devcount, (unsigned int) devices[i], devname); @@ -82,7 +82,6 @@ test_multi_audio(SDL_AudioDeviceID *devices, int devcount) SDL_Log("done."); SDL_DestroyAudioStream(stream); } - SDL_free(devname); stream = NULL; } diff --git a/test/testsurround.c b/test/testsurround.c index 23a522c65..e501101f2 100644 --- a/test/testsurround.c +++ b/test/testsurround.c @@ -191,12 +191,11 @@ int main(int argc, char *argv[]) for (i = 0; i < devcount; i++) { SDL_AudioStream *stream = NULL; - char *devname = SDL_GetAudioDeviceName(devices[i]); + const char *devname = SDL_GetAudioDeviceName(devices[i]); int j; SDL_AudioSpec spec; SDL_Log("Testing audio device: %s\n", devname); - SDL_free(devname); if (SDL_GetAudioDeviceFormat(devices[i], &spec, NULL) != 0) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "SDL_GetAudioDeviceFormat() failed: %s\n", SDL_GetError());