[v1 1/1] dinput: Sort virtual joysticks by name on the Mac
Ken Thomases
ken at codeweavers.com
Sun Jul 17 16:08:41 CDT 2016
On Jul 16, 2016, at 3:15 AM, David Lawrie <david.dljunk at gmail.com> wrote:
>
> Tested on OS X 10.10.5.
>
> Signed-off-by: David Lawrie <david.dljunk at gmail.com>
> ---
> dlls/dinput/joystick_osx.c | 35 ++++++++++++++++++++++++++++++++---
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/dinput/joystick_osx.c b/dlls/dinput/joystick_osx.c
> index 637692b..22bdc4f 100644
> --- a/dlls/dinput/joystick_osx.c
> +++ b/dlls/dinput/joystick_osx.c
> @@ -193,6 +193,26 @@ static long get_device_property_long(IOHIDDeviceRef device, CFStringRef key)
> return result;
> }
>
> +static CFStringRef get_device_name(IOHIDDeviceRef device)
> +{
> + CFTypeRef ref;
> + CFStringRef name = CFSTR("Default Name");
You can probably do better for a fallback. For example, you can format the vendor ID and product ID into a string. For an extreme take on this, see the Copy_DeviceName() function in Apple's HID Calibrator sample code:
https://developer.apple.com/library/mac/samplecode/HID_Calibrator/Listings/HID_Calibrator_IOHIDDeviceWindowCtrl_m.html
> +
> + if(device)
> + {
> + assert(IOHIDDeviceGetTypeID() == CFGetTypeID(device));
> +
> + ref = IOHIDDeviceGetProperty(device, CFSTR(kIOHIDProductKey));
> +
> + if (ref && CFStringGetTypeID() == CFGetTypeID(ref))
> + name = ref;
> + else if (ref)
> + name = CFCopyDescription(ref);
This introduces a memory-management problem, a.k.a. a leak. This copy is never released.
If you follow my suggestion above for formatting a string as a fallback if the product name isn't available, you'd also have to release that.
So, you would change this function's name to copy_device_name() and make sure all code paths return an owned reference to the caller, which the caller has to release. Basically, in the case where IOHIDDeviceGetProperty() gives you a string, you'd have to copy it to match the other code paths. Of course, you'd have to make sure the callers do, in fact, release the references.
> + }
> +
> + return name;
> +}
> +
> static long get_device_location_ID(IOHIDDeviceRef device)
> {
> return get_device_property_long(device, CFSTR(kIOHIDLocationIDKey));
> @@ -203,7 +223,15 @@ static void copy_set_to_array(const void *value, void *context)
> CFArrayAppendValue(context, value);
> }
>
> -static CFComparisonResult device_location_comparator(const void *val1, const void *val2, void *context)
> +static CFComparisonResult device_name_comparator(IOHIDDeviceRef device1, IOHIDDeviceRef device2)
> +{
> + CFStringRef name1 = get_device_name(device1), name2 = get_device_name(device2);
> + CFStringCompareFlags compareOptions = kCFCompareForcedOrdering | kCFCompareNumerically;
Any reason you've added this separate variable instead of just inlining the options in the call?
> +
> + return CFStringCompare(name1, name2, compareOptions);
> +}
-Ken
More information about the wine-devel
mailing list