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.)
This commit is contained in:
Ryan C. Gordon
2024-06-01 22:05:21 -04:00
parent b1f3682216
commit e23257307e
51 changed files with 262 additions and 123 deletions

View File

@@ -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)

View File

@@ -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;

View File

@@ -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
}

View File

@@ -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;

View File

@@ -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);

View File

@@ -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;

View File

@@ -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;

View File

@@ -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;

View File

@@ -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)

View File

@@ -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);

View File

@@ -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));
}
}

View File

@@ -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]));

View File

@@ -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--;

View File

@@ -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<SDL_WinRT_Path, string> UTF8PathMap;

View File

@@ -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);
}

View File

@@ -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;
}

View File

@@ -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);

View File

@@ -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 = {

View File

@@ -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

View File

@@ -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();

View File

@@ -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);
}
}

View File

@@ -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;

View File

@@ -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) {