Re: [v2 1/3] dinput: Print location ID in debugstr_device on the Mac

[Dmitry Timoshkov]

David Lawrie  wrote:
> +static Boolean get_device_property_long(IOHIDDeviceRef in_device, CFStringRef in_key, long * out_value)
> +{
> - Boolean result = FALSE;
> - CFTypeRef type;
> -
> - if (in_device)
> - {
> -
>    assert(IOHIDDeviceGetTypeID() == CFGetTypeID(in_device));
> -
> -
>    type = IOHIDDeviceGetProperty(in_device, in_key);
> -
> -
>    if (type)
> -
>    {
> -
>        if (CFNumberGetTypeID() == CFGetTypeID(type))
> -
>            result = CFNumberGetValue((CFNumberRef)type, kCFNumberLongType, out_value);
> -
>    }
> - }
> - return result;
>
> +}
> -
>
> +long get_device_location_ID(IOHIDDeviceRef device)
> +{
> - long result = 0;
> - get_device_property_long(device, CFSTR(kIOHIDLocationIDKey), &result);
> - return result;
>
> +}

The helper should be 'static'. Also, probably you should return value from
the helper directly since you don't check for the boolean result anyway.

Same comment applies for a similar winejoystick.drv patch.

[v2 1/3] dinput: Print location ID in debugstr_device on the Mac

[DavidL]

Ah, okay ... I have a couple of other new helper functions, I should
probably make sure they are static as well. Thanks!

On Thursday, June 30, 2016, Dmitry Timoshkov  wrote:
> David Lawrie <[e-mail] javascript:;> wrote:
> > +static Boolean get_device_property_long(IOHIDDeviceRef in_device,
>
> CFStringRef in_key, long * out_value)
> > +{
> > - Boolean result = FALSE;
> > - CFTypeRef type;
> > -
> > - if (in_device)
> > - {
> > -
> >    assert(IOHIDDeviceGetTypeID() == CFGetTypeID(in_device));
> > -
> > -
> >    type = IOHIDDeviceGetProperty(in_device, in_key);
> > -
> > -
> >    if (type)
> > -
> >    {
> > -
> >        if (CFNumberGetTypeID() == CFGetTypeID(type))
> > -
> >            result = CFNumberGetValue((CFNumberRef)type,
>
> kCFNumberLongType, out_value);
> > -
> >    }
> > - }
> > - return result;
> >
> > +}
> > -
> >
> > +long get_device_location_ID(IOHIDDeviceRef device)
> > +{
> > - long result = 0;
> > - get_device_property_long(device, CFSTR(kIOHIDLocationIDKey),
>
> &result);
> > - return result;
> >
> > +}
>
> The helper should be 'static'. Also, probably you should return value from
> the helper directly since you don't check for the boolean result anyway.
>
> Same comment applies for a similar winejoystick.drv patch.
>
> --
> Dmitry.