Modernize memory management of Core Foundation pointers

src/SFML/Window/macOS/HIDInputManager.hpp

@@ -30,6 +30,7 @@
 ////////////////////////////////////////////////////////////
 #include <SFML/Window/JoystickImpl.hpp>
 #include <SFML/Window/Keyboard.hpp>
+#include <SFML/Window/macOS/Utils.hpp>
 
 #include <IOKit/hid/IOHIDDevice.h>
 #include <IOKit/hid/IOHIDManager.h>
@@ -243,10 +244,10 @@ private:
     ///
     /// \param page  HID page like kHIDPage_GenericDesktop
     /// \param usage HID usage page like kHIDUsage_GD_Keyboard or kHIDUsage_GD_Mouse
-    /// \return a retained, non-empty CFSetRef of IOHIDDeviceRef or a null pointer
+    /// \return a retained, non-empty __CFSet pointer of IOHIDDeviceRef or a null pointer
     ///
     ////////////////////////////////////////////////////////////
-    CFSetRef copyDevices(std::uint32_t page, std::uint32_t usage);
+    CFPtr<const __CFSet> copyDevices(std::uint32_t page, std::uint32_t usage);
 
     ////////////////////////////////////////////////////////////
     /// \brief Check if a key is pressed
@@ -286,8 +287,8 @@ private:
     ////////////////////////////////////////////////////////////
     // Member data
     ////////////////////////////////////////////////////////////
-    IOHIDManagerRef m_manager{};         ///< Underlying HID Manager
+    CFPtr<__IOHIDManager> m_manager{};         ///< Underlying HID Manager
-    bool            m_keysInitialized{}; ///< Has initializeKeyboard been called at least once?
+    bool                  m_keysInitialized{}; ///< Has initializeKeyboard been called at least once?
     EnumArray<Keyboard::Scancode, IOHIDElements, Keyboard::ScancodeCount> m_keys; ///< All the keys on any connected keyboard
     EnumArray<Keyboard::Key, Keyboard::Scancode, Keyboard::KeyCount> m_keyToScancodeMapping{}; ///< Mapping from Key to Scancode
     EnumArray<Keyboard::Scancode, Keyboard::Key, Keyboard::ScancodeCount> m_scancodeToKeyMapping{}; ///< Mapping from Scancode to Key

src/SFML/Window/macOS/HIDInputManager.mm

@@ -78,14 +78,12 @@ CFDictionaryRef HIDInputManager::copyDevicesMask(std::uint32_t page, std::uint32
                                                             &kCFTypeDictionaryValueCallBacks);
 
     // Add the page value.
-    CFNumberRef value = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &page);
+    auto value = CFPtr<const __CFNumber>(CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &page));
-    CFDictionarySetValue(dict, CFSTR(kIOHIDDeviceUsagePageKey), value);
+    CFDictionarySetValue(dict, CFSTR(kIOHIDDeviceUsagePageKey), value.get());
-    CFRelease(value);
 
     // Add the usage value (which is only valid if page value exists).
-    value = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &usage);
+    value = CFPtr<const __CFNumber>(CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &usage));
-    CFDictionarySetValue(dict, CFSTR(kIOHIDDeviceUsageKey), value);
+    CFDictionarySetValue(dict, CFSTR(kIOHIDDeviceUsageKey), value.get());
-    CFRelease(value);
 
     return dict;
 }
@@ -712,8 +710,8 @@ String HIDInputManager::getDescription(Keyboard::Scancode code)
 HIDInputManager::HIDInputManager()
 {
     // Create an HID Manager reference
-    m_manager                 = IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDOptionsTypeNone);
+    m_manager                 = CFPtr<__IOHIDManager>(IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDOptionsTypeNone));
-    const IOReturn openStatus = IOHIDManagerOpen(m_manager, kIOHIDOptionsTypeNone);
+    const IOReturn openStatus = IOHIDManagerOpen(m_manager.get(), kIOHIDOptionsTypeNone);
 
     if (openStatus != kIOReturnSuccess)
     {
@@ -752,7 +750,7 @@ void HIDInputManager::initializeKeyboard()
     // in approximately constant time.
 
     // Get only keyboards
-    CFSetRef underlying = copyDevices(kHIDPage_GenericDesktop, kHIDUsage_GD_Keyboard);
+    const auto underlying = copyDevices(kHIDPage_GenericDesktop, kHIDUsage_GD_Keyboard);
     if (underlying == nullptr)
     {
         err() << "No keyboard detected by the HID manager!" << std::endl;
@@ -760,12 +758,10 @@ void HIDInputManager::initializeKeyboard()
         return;
     }
 
-    auto* const keyboards = static_cast<NSSet*>(underlying); // Toll-Free Bridge
+    auto* const keyboards = static_cast<NSSet*>(underlying.get()); // Toll-Free Bridge
-    for (id keyboard in keyboards)                           // NOLINT(cppcoreguidelines-init-variables)
+    for (id keyboard in keyboards)                                 // NOLINT(cppcoreguidelines-init-variables)
         loadKeyboard(static_cast<IOHIDDeviceRef>(keyboard));
 
-    CFRelease(underlying);
-
     ////////////////////////////////////////////////////////////
     // At this point m_keys is filled with as many IOHIDElementRef as possible
 }
@@ -774,22 +770,21 @@ void HIDInputManager::initializeKeyboard()
 ////////////////////////////////////////////////////////////
 void HIDInputManager::loadKeyboard(IOHIDDeviceRef keyboard)
 {
-    CFArrayRef underlying = IOHIDDeviceCopyMatchingElements(keyboard, nullptr, kIOHIDOptionsTypeNone);
+    const auto underlying = CFPtr<const __CFArray>(
-    if ((underlying == nullptr) || (CFArrayGetCount(underlying) == 0))
+        IOHIDDeviceCopyMatchingElements(keyboard, nullptr, kIOHIDOptionsTypeNone));
+    if ((underlying == nullptr) || (CFArrayGetCount(underlying.get()) == 0))
     {
         err() << "Detected a keyboard without any keys." << std::endl;
         return;
     }
 
-    auto* const keys = static_cast<NSArray*>(underlying); // Toll-Free Bridge
+    auto* const keys = static_cast<NSArray*>(underlying.get()); // Toll-Free Bridge
-    for (id key in keys)                                  // NOLINT(cppcoreguidelines-init-variables)
+    for (id key in keys)                                        // NOLINT(cppcoreguidelines-init-variables)
     {
         auto* elem = static_cast<IOHIDElementRef>(key);
         if (IOHIDElementGetUsagePage(elem) == kHIDPage_KeyboardOrKeypad)
             loadKey(elem);
     }
-
-    CFRelease(underlying);
 }
 
 
@@ -814,13 +809,12 @@ void HIDInputManager::buildMappings()
     m_scancodeToKeyMapping.fill(Keyboard::Key::Unknown);
 
     // Get the current keyboard layout
-    TISInputSourceRef tis  = TISCopyCurrentKeyboardLayoutInputSource();
+    const auto tis         = CFPtr<__TISInputSource>(TISCopyCurrentKeyboardLayoutInputSource());
-    const auto* layoutData = static_cast<CFDataRef>(TISGetInputSourceProperty(tis, kTISPropertyUnicodeKeyLayoutData));
+    const auto* layoutData = static_cast<CFDataRef>(TISGetInputSourceProperty(tis.get(), kTISPropertyUnicodeKeyLayoutData));
 
     if (layoutData == nullptr)
     {
         err() << "Cannot get the keyboard layout\n";
-        CFRelease(tis);
         return;
     }
 
@@ -905,8 +899,6 @@ void HIDInputManager::buildMappings()
             m_keyToScancodeMapping[code] = scan;
         m_scancodeToKeyMapping[scan] = code;
     }
-
-    CFRelease(tis);
 }
 
 
@@ -925,10 +917,7 @@ void HIDInputManager::keyboardChanged(CFNotificationCenterRef /* center */,
 ////////////////////////////////////////////////////////////
 void HIDInputManager::freeUp()
 {
-    if (m_manager != nil)
+    m_manager.reset();
-        CFRelease(m_manager);
-
-    m_manager = nil;
 
     if (m_keysInitialized)
     {
@@ -945,26 +934,20 @@ void HIDInputManager::freeUp()
 
 
 ////////////////////////////////////////////////////////////
-CFSetRef HIDInputManager::copyDevices(std::uint32_t page, std::uint32_t usage)
+CFPtr<const __CFSet> HIDInputManager::copyDevices(std::uint32_t page, std::uint32_t usage)
 {
     // Filter and keep only the requested devices
-    CFDictionaryRef mask = copyDevicesMask(page, usage);
+    const auto mask = CFPtr<const __CFDictionary>(copyDevicesMask(page, usage));
 
-    IOHIDManagerSetDeviceMatching(m_manager, mask);
+    IOHIDManagerSetDeviceMatching(m_manager.get(), mask.get());
 
-    CFRelease(mask);
+    auto devices = CFPtr<const __CFSet>(IOHIDManagerCopyDevices(m_manager.get()));
-    mask = nil;
-
-    CFSetRef devices = IOHIDManagerCopyDevices(m_manager);
     if (devices == nullptr)
         return nullptr;
 
     // Is there at least one device?
-    if (CFSetGetCount(devices) < 1)
+    if (CFSetGetCount(devices.get()) < 1)
-    {
-        CFRelease(devices);
         return nullptr;
-    }
 
     return devices;
 }

src/SFML/Window/macOS/HIDJoystickManager.cpp

@@ -61,10 +61,9 @@ unsigned int HIDJoystickManager::getJoystickCount()
 
 
 ////////////////////////////////////////////////////////////
-CFSetRef HIDJoystickManager::copyJoysticks()
+CFPtr<const __CFSet> HIDJoystickManager::copyJoysticks()
 {
-    CFSetRef devices = IOHIDManagerCopyDevices(m_manager);
+    return CFPtr<const __CFSet>(IOHIDManagerCopyDevices(m_manager));
-    return devices;
 }
 
 
@@ -73,17 +72,17 @@ HIDJoystickManager::HIDJoystickManager()
 {
     m_manager = IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDOptionsTypeNone);
 
-    CFDictionaryRef mask0 = HIDInputManager::copyDevicesMask(kHIDPage_GenericDesktop, kHIDUsage_GD_Joystick);
+    const auto mask0 = CFPtr<const __CFDictionary>(
+        HIDInputManager::copyDevicesMask(kHIDPage_GenericDesktop, kHIDUsage_GD_Joystick));
 
-    CFDictionaryRef mask1 = HIDInputManager::copyDevicesMask(kHIDPage_GenericDesktop, kHIDUsage_GD_GamePad);
+    const auto mask1 = CFPtr<const __CFDictionary>(
+        HIDInputManager::copyDevicesMask(kHIDPage_GenericDesktop, kHIDUsage_GD_GamePad));
 
-    std::array maskArray = {mask0, mask1};
+    std::array maskArray = {mask0.get(), mask1.get()};
-    CFArrayRef mask = CFArrayCreate(nullptr, reinterpret_cast<const void**>(maskArray.data()), maskArray.size(), nullptr);
+    const auto mask      = CFPtr<const __CFArray>(
+        CFArrayCreate(nullptr, reinterpret_cast<const void**>(maskArray.data()), maskArray.size(), nullptr));
 
-    IOHIDManagerSetDeviceMatchingMultiple(m_manager, mask);
+    IOHIDManagerSetDeviceMatchingMultiple(m_manager, mask.get());
-    CFRelease(mask);
-    CFRelease(mask1);
-    CFRelease(mask0);
 
 
     IOHIDManagerRegisterDeviceMatchingCallback(m_manager, pluggedIn, this);

src/SFML/Window/macOS/HIDJoystickManager.hpp

@@ -75,10 +75,10 @@ public:
     ////////////////////////////////////////////////////////////
     /// \brief Copy the devices associated with this HID manager
     ///
-    /// \return a retained CFSetRef of IOHIDDeviceRef or a null pointer
+    /// \return a retained __CFSet pointer of IOHIDDeviceRef or a null pointer
     ///
     ////////////////////////////////////////////////////////////
-    CFSetRef copyJoysticks();
+    CFPtr<const __CFSet> copyJoysticks();
 
 private:
     ////////////////////////////////////////////////////////////

src/SFML/Window/macOS/InputImpl.mm

@@ -212,12 +212,12 @@ void setMousePosition(Vector2i position)
     const CGPoint pos   = CGPointMake(position.x / scale, position.y / scale);
 
     // Place the cursor.
-    CGEventRef event = CGEventCreateMouseEvent(nullptr,
+    const auto event = CFPtr<__CGEvent>(
-                                               kCGEventMouseMoved,
+        CGEventCreateMouseEvent(nullptr,
-                                               pos,
+                                kCGEventMouseMoved,
-                                               /* we don't care about this: */ kCGMouseButtonLeft);
+                                pos,
-    CGEventPost(kCGHIDEventTap, event);
+                                /* we don't care about this: */ kCGMouseButtonLeft));
-    CFRelease(event);
+    CGEventPost(kCGHIDEventTap, event.get());
     // This is a workaround to deprecated CGSetLocalEventsSuppressionInterval.
 }
 

src/SFML/Window/macOS/JoystickImpl.cpp

@@ -132,15 +132,15 @@ bool JoystickImpl::isConnected(unsigned int index)
         if (connectedCount > openedCount)
         {
             // Get all devices
-            CFSetRef devices = HIDJoystickManager::getInstance().copyJoysticks();
+            const auto devices = CFPtr<const __CFSet>(HIDJoystickManager::getInstance().copyJoysticks());
 
             if (devices != nullptr)
             {
-                const CFIndex size = CFSetGetCount(devices);
+                const CFIndex size = CFSetGetCount(devices.get());
                 if (size > 0)
                 {
                     std::vector<CFTypeRef> array(static_cast<std::size_t>(size)); // array of IOHIDDeviceRef
-                    CFSetGetValues(devices, array.data());
+                    CFSetGetValues(devices.get(), array.data());
 
                     // If there exists a device d s.t. there is no j s.t.
                     // m_locationIDs[j] == d's location then we have a new device.
@@ -166,8 +166,6 @@ bool JoystickImpl::isConnected(unsigned int index)
                         }
                     }
                 }
-
-                CFRelease(devices);
             }
         }
     }
@@ -185,14 +183,14 @@ bool JoystickImpl::open(unsigned int index)
     const Location deviceLoc = m_locationIDs[index]; // The device we need to load
 
     // Get all devices
-    CFSetRef devices = HIDJoystickManager::getInstance().copyJoysticks();
+    const auto devices = HIDJoystickManager::getInstance().copyJoysticks();
     if (devices == nullptr)
         return false;
 
     // Get a usable copy of the joysticks devices.
-    const CFIndex          joysticksCount = CFSetGetCount(devices);
+    const CFIndex          joysticksCount = CFSetGetCount(devices.get());
     std::vector<CFTypeRef> devicesArray(static_cast<std::size_t>(joysticksCount));
-    CFSetGetValues(devices, devicesArray.data());
+    CFSetGetValues(devices.get(), devicesArray.data());
 
     // Get the desired joystick.
     IOHIDDeviceRef self = nil;
@@ -204,29 +202,23 @@ bool JoystickImpl::open(unsigned int index)
     }
 
     if (self == nil)
-    {
-        CFRelease(devices);
         return false;
-    }
 
     m_identification.name      = getDeviceString(self, CFSTR(kIOHIDProductKey), m_index);
     m_identification.vendorId  = getDeviceUint(self, CFSTR(kIOHIDVendorIDKey), m_index);
     m_identification.productId = getDeviceUint(self, CFSTR(kIOHIDProductIDKey), m_index);
 
     // Get a list of all elements attached to the device.
-    CFArrayRef elements = IOHIDDeviceCopyMatchingElements(self, nullptr, kIOHIDOptionsTypeNone);
+    const auto elements = CFPtr<const __CFArray>(IOHIDDeviceCopyMatchingElements(self, nullptr, kIOHIDOptionsTypeNone));
 
     if (elements == nullptr)
-    {
-        CFRelease(devices);
         return false;
-    }
 
     // Go through all connected elements.
-    const CFIndex elementsCount = CFArrayGetCount(elements);
+    const CFIndex elementsCount = CFArrayGetCount(elements.get());
     for (int i = 0; i < elementsCount; ++i)
     {
-        auto* element = static_cast<IOHIDElementRef>(const_cast<void*>(CFArrayGetValueAtIndex(elements, i)));
+        auto* element = static_cast<IOHIDElementRef>(const_cast<void*>(CFArrayGetValueAtIndex(elements.get(), i)));
         switch (IOHIDElementGetUsagePage(element))
         {
             case kHIDPage_GenericDesktop:
@@ -331,9 +323,6 @@ bool JoystickImpl::open(unsigned int index)
     // Axis X (for example) and we want to keep only the last one. To prevent
     // leaking we retain objects 'only' now.
 
-    CFRelease(devices);
-    CFRelease(elements);
-
     return true;
 }
 
@@ -406,14 +395,14 @@ JoystickState JoystickImpl::update()
     const Location selfLoc = m_locationIDs[m_index];
 
     // Get all devices
-    CFSetRef devices = HIDJoystickManager::getInstance().copyJoysticks();
+    auto devices = HIDJoystickManager::getInstance().copyJoysticks();
     if (devices == nullptr)
         return disconnectedState;
 
     // Get a usable copy of the joysticks devices.
-    const CFIndex          joysticksCount = CFSetGetCount(devices);
+    const CFIndex          joysticksCount = CFSetGetCount(devices.get());
     std::vector<CFTypeRef> devicesArray(static_cast<std::size_t>(joysticksCount));
-    CFSetGetValues(devices, devicesArray.data());
+    CFSetGetValues(devices.get(), devicesArray.data());
 
     // Search for it
     bool found = false;
@@ -424,9 +413,6 @@ JoystickState JoystickImpl::update()
             found = true;
     }
 
-    // Release unused stuff
-    CFRelease(devices);
-
     // If not found we consider it disconnected
     if (!found)
         return disconnectedState;

src/SFML/Window/macOS/Utils.hpp

@@ -0,0 +1,54 @@
+////////////////////////////////////////////////////////////
+//
+// SFML - Simple and Fast Multimedia Library
+// Copyright (C) 2007-2024 Laurent Gomila (laurent@sfml-dev.org)
+//
+// This software is provided 'as-is', without any express or implied warranty.
+// In no event will the authors be held liable for any damages arising from the use of this software.
+//
+// Permission is granted to anyone to use this software for any purpose,
+// including commercial applications, and to alter it and redistribute it freely,
+// subject to the following restrictions:
+//
+// 1. The origin of this software must not be misrepresented;
+//    you must not claim that you wrote the original software.
+//    If you use this software in a product, an acknowledgment
+//    in the product documentation would be appreciated but is not required.
+//
+// 2. Altered source versions must be plainly marked as such,
+//    and must not be misrepresented as being the original software.
+//
+// 3. This notice may not be removed or altered from any source distribution.
+//
+////////////////////////////////////////////////////////////
+
+#pragma once
+
+////////////////////////////////////////////////////////////
+// Headers
+////////////////////////////////////////////////////////////
+#include <memory>
+
+
+namespace sf::priv
+{
+////////////////////////////////////////////////////////////
+/// \brief Class for freeing Core Foundation pointers
+///
+////////////////////////////////////////////////////////////
+struct CFDeleter
+{
+    template <typename T>
+    void operator()(T* data) const
+    {
+        CFRelease(data);
+    }
+};
+
+////////////////////////////////////////////////////////////
+/// \brief Class template for wrapping owning raw pointers from Core Foundation
+///
+////////////////////////////////////////////////////////////
+template <typename T>
+using CFPtr = std::unique_ptr<T, CFDeleter>;
+} // namespace sf::priv

src/SFML/Window/macOS/VideoModeImpl.cpp

@@ -27,6 +27,7 @@
 // Headers
 ////////////////////////////////////////////////////////////
 #include <SFML/Window/VideoModeImpl.hpp>
+#include <SFML/Window/macOS/Utils.hpp>
 #include <SFML/Window/macOS/cg_sf_conversion.hpp>
 
 #include <SFML/System/Err.hpp>
@@ -43,7 +44,7 @@ std::vector<VideoMode> VideoModeImpl::getFullscreenModes()
     std::vector<VideoMode> modes;
 
     // Retrieve all modes available for main screen only.
-    CFArrayRef cgmodes = CGDisplayCopyAllDisplayModes(CGMainDisplayID(), nullptr);
+    const auto cgmodes = CFPtr<const __CFArray>(CGDisplayCopyAllDisplayModes(CGMainDisplayID(), nullptr));
 
     if (cgmodes == nullptr)
     {
@@ -54,10 +55,10 @@ std::vector<VideoMode> VideoModeImpl::getFullscreenModes()
     const VideoMode desktop = getDesktopMode();
 
     // Loop on each mode and convert it into a sf::VideoMode object.
-    const CFIndex modesCount = CFArrayGetCount(cgmodes);
+    const CFIndex modesCount = CFArrayGetCount(cgmodes.get());
     for (CFIndex i = 0; i < modesCount; ++i)
     {
-        auto* cgmode = static_cast<CGDisplayModeRef>(const_cast<void*>(CFArrayGetValueAtIndex(cgmodes, i)));
+        auto* cgmode = static_cast<CGDisplayModeRef>(const_cast<void*>(CFArrayGetValueAtIndex(cgmodes.get(), i)));
 
         const VideoMode mode = convertCGModeToSFMode(cgmode);
 
@@ -70,9 +71,6 @@ std::vector<VideoMode> VideoModeImpl::getFullscreenModes()
             modes.push_back(mode);
     }
 
-    // Clean up memory.
-    CFRelease(cgmodes);
-
     return modes;
 }
 

src/SFML/Window/macOS/cg_sf_conversion.mm

@@ -27,6 +27,7 @@
 // Headers
 ////////////////////////////////////////////////////////////
 #import <SFML/Window/macOS/Scaling.h>
+#include <SFML/Window/macOS/Utils.hpp>
 #include <SFML/Window/macOS/cg_sf_conversion.hpp>
 
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
@@ -37,21 +38,16 @@ namespace sf::priv
 ////////////////////////////////////////////////////////////
 unsigned int modeBitsPerPixel(CGDisplayModeRef mode)
 {
-    unsigned int bpp = 0; // no match
-
     // Compare encoding.
-    CFStringRef pixEnc = CGDisplayModeCopyPixelEncoding(mode);
+    const auto pixEnc = CFPtr<const __CFString>(CGDisplayModeCopyPixelEncoding(mode));
-    if (CFStringCompare(pixEnc, CFSTR(IO32BitDirectPixels), kCFCompareCaseInsensitive) == kCFCompareEqualTo)
+    if (CFStringCompare(pixEnc.get(), CFSTR(IO32BitDirectPixels), kCFCompareCaseInsensitive) == kCFCompareEqualTo)
-        bpp = 32;
+        return 32;
-    else if (CFStringCompare(pixEnc, CFSTR(IO16BitDirectPixels), kCFCompareCaseInsensitive) == kCFCompareEqualTo)
+    if (CFStringCompare(pixEnc.get(), CFSTR(IO16BitDirectPixels), kCFCompareCaseInsensitive) == kCFCompareEqualTo)
-        bpp = 16;
+        return 16;
-    else if (CFStringCompare(pixEnc, CFSTR(IO8BitIndexedPixels), kCFCompareCaseInsensitive) == kCFCompareEqualTo)
+    if (CFStringCompare(pixEnc.get(), CFSTR(IO8BitIndexedPixels), kCFCompareCaseInsensitive) == kCFCompareEqualTo)
-        bpp = 8;
+        return 8;
 
-    // Clean up memory.
+    return 0;
-    CFRelease(pixEnc);
-
-    return bpp;
 }