macdrv: implement getting and setting the screen saver state.

Ken Thomases ken at codeweavers.com
Sun Jan 20 23:34:01 CST 2013


On Jan 20, 2013, at 11:47 AM, C.W. Betts wrote:

> On Jan 20, 2013, at 12:38 AM, Ken Thomases <ken at codeweavers.com> wrote:
> 
>> On Jan 19, 2013, at 4:08 PM, C.W. Betts wrote:
>> 
>>> +                    CFNumberRef count = CFDictionaryGetValue(assertsionStats, kIOPMAssertionTypeNoDisplaySleep);
>>> +                    CFNumberRef count2 = NULL;
>>> +                    long longCount = 0;
>>> +                    long longCount2 = 0;
>>> +                    CFNumberGetValue(count, kCFNumberLongType, &longCount);
>>> +                    count2 = CFDictionaryGetValue(assertsionStats, kIOPMAssertionTypePreventUserIdleDisplaySleep);
>>> +                    if(count2)
>>> +                        CFNumberGetValue(count2, kCFNumberLongType, &longCount2);
>> 
>> Why are the two assertion types handled differently here?  You get the value in the initializer in one case but not in the other.  You check if the CFNumber pointer is NULL in one case but not the other.  They should probably both be handled the same way.
> On Lion and later, kIOPMAssertionTypeNoDisplaySleep is deprecated in favor of kIOPMAssertionTypePreventUserIdleDisplaySleep. This catches both: I don't know if OS X Lion and later automatically maps kIOPMAssertionTypeNoDisplaySleep to kIOPMAssertionTypePreventUserIdleDisplaySleep, so this takes care of both.

I understand why you're checking both.  My question was why the code wasn't similar for checking each case.  You check them both, which is good, but you check each in a slightly different way, which is confusing and inconsistent.  It's also error-prone for the possible future where CFDictionaryGetValue(..., kIOPMAssertionTypeNoDisplaySleep) might return NULL.  Should that happen, CFNumberGetValue() would crash.

-Ken




More information about the wine-devel mailing list