On Thu, Jul 21, 2016 at 10:41 PM, Ken Thomases <ken@codeweavers.com> wrote:

Hi,

On Jul 20, 2016, at 2:15 AM, David Lawrie <david.dljunk@gmail.com> wrote:
>
> +static CFStringRef copy_device_name(IOHIDDeviceRef device)
> +{
> +�� �� CFTypeRef ref_name, ref_man;
> +�� �� CFStringRef name;
> +�� �� long prodID;

I prefer variables to be declared at the narrowest scope in which they're used.

> +
> +�� �� if (device)
> +�� �� {
> +�� �� �� �� assert(IOHIDDeviceGetTypeID() == CFGetTypeID(device));
> +
> +�� �� �� �� ref_name = IOHIDDeviceGetProperty(device, CFSTR(kIOHIDProductKey));
> +
> +�� �� �� �� if (ref_name && CFStringGetTypeID() == CFGetTypeID(ref_name))
> +�� �� �� �� �� �� name = CFStringCreateCopy(kCFAllocatorDefault,ref_name);
> +�� �� �� �� else
> +�� �� �� �� {
> +�� �� �� �� �� �� ref_man = IOHIDDeviceGetProperty(device, CFSTR(kIOHIDManufacturerKey));

I think it's simpler to use kIOHIDVendorIDKey with get_device_property_long().�� Judging by the HID Calibrator code, kIOHIDManufacturerKey is not reliably available, but kIOHIDVendorIDKey should be.�� It's also pretty fundamental to USB/HID.

> +�� �� �� �� �� �� prodID = get_device_property_long(device, CFSTR(kIOHIDProductIDKey));
> +
> +�� �� �� �� �� �� if(ref_man && CFStringGetTypeID() == CFGetTypeID(ref_man))
> +�� �� �� �� �� �� �� �� name = CFStringCreateWithFormat(kCFAllocatorDefault, NULL, CFSTR("%@ %lu"), ref_man, prodID);

Better to format vendor and product IDs using "0x%04lx" since that's how they are usually represented (e.g. within HID Calibrator windows and System Profiler).�� (Yes, I realize that Copy_DeviceName() which I cited previously formats them as decimal.)

Thanks for persevering in the face of my picky-ness. :)

-Ken