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

[Ken Thomases]

On Jun 24, 2016, at 8:33 PM, David Lawrie <david.dljunk at gmail.com> 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