Re: [v1 1/2] dinput/joystick_osx.c: debugstr_device prints location ID

[Ken Thomases]

On Jun 24, 2016, at 8:33 PM, David Lawrie  wrote:
> +static Boolean IOHIDDevice_GetLongProperty(IOHIDDeviceRef inIOHIDDeviceRef, CFStringRef inKey, long * outValue)

Don't name your functions as though they were Apple's.  Apple "owns" the namespace of identifiers starting with "IOHIDDevice".  In general, just use simple lowercase-and-underscores names unless there's a strong reason not to.  Something like "get_device_property_long".

Same goes for the parameters: "key" and "out_value" or the like.
> +{
> - Boolean result = FALSE;
> - CFTypeRef tCFTypeRef;

Don't use this naming convention either (is that Hungarian notation?).  Why is the type repeated in the variable name?  What does the "t" prefix signify.  Just call this "value" or something.
> -
> - if (inIOHIDDeviceRef) {
> -
>    assert(IOHIDDeviceGetTypeID() == CFGetTypeID(inIOHIDDeviceRef));
> -
> -
>    tCFTypeRef = IOHIDDeviceGetProperty(inIOHIDDeviceRef, inKey);
> -
> -
>    if ( tCFTypeRef ) {
> -
>        /* if this is a number */
> -
>        if (CFNumberGetTypeID() == CFGetTypeID(tCFTypeRef)) {
> -
>            /* get it's value */

What do these comments add?
> -
>            result = CFNumberGetValue((CFNumberRef)tCFTypeRef, kCFNumberSInt32Type, outValue);

"long" and "SInt32" don't match reliably.  Why not kCFNumberLongType?
> +long IOHIDDevice_GetLocationID(IOHIDDeviceRef inIOHIDDeviceRef)

Naming conventions here, too.
> +{
> - long result = 0;
> - (void)IOHIDDevice_GetLongProperty(inIOHIDDeviceRef, CFSTR(kIOHIDLocationIDKey), &result);

I don't think we generally use the cast-to-void thing for unused return values in Wine.

-Ken