[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [v1 1/2] dinput/joystick_osx.c: debugstr_device prints location ID
From: Ken Thomases <ken () codeweavers ! com>
Date: 2016-06-29 17:27:51
Message-ID: 1417808C-0ACE-4B77-8C5F-DA60E763682F () codeweavers ! com
[Download RAW message or body]
On Jun 24, 2016, at 8:33 PM, David Lawrie <david.dljunk@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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic