dixSetPrivate regression

[Jeremy Huddleston]

Ok, that seems to get past the problem in XQuartz, but we're still failing in xtest  ...

Assertion failed: (key->initialized), function dixGetPrivateAddr, file ../include/privates.h, line 122.

(gdb) bt
#0  0x00007fff818b9b6e in __semwait_signal_nocancel ()
#1  0x00007fff818b9a70 in nanosleep$NOCANCEL ()
#2  0x00007fff819163c6 in usleep$NOCANCEL ()
#3  0x00007fff8193597c in abort ()
#4  0x00007fff819229b4 in __assert_rtn ()
#5  0x0000000100066419 in dixGetPrivateAddr (privates=0x10f962020, key=0x1002a8b80) at privates.h:122
#6  0x00000001000663ca in dixSetPrivate (privates=0x10f962020, key=0x1002a8b80, val=0x2) at privates.h:148
#7  0x00000001000661eb in AllocXTestDevice (client=0x104e00650, name=0x1002240c2 "Virtual core", ptr=0x1002b5a20, keybd=0x1002b5a28, master_ptr=0x10f95f800, master_keybd=0x10f95fe00) at xtest.c:651
#8  0x0000000100065fc7 in InitXTestDevices () at xtest.c:601
#9  0x000000010013b4ea in InitCoreDevices () at devices.c:680
#10 0x00000001001321da in dix_main (argc=4, argv=0x7fff5fbfdbc0, envp=0x7fff5fbfda70) at main.c:260
#11 0x0000000100018f0b in server_thread (arg=0x103309320) at quartzStartup.c:63
#12 0x00007fff81880456 in _pthread_start ()
#13 0x00007fff81880309 in thread_start ()



On Jun 6, 2010, at 21:33, Keith Packard wrote:

> On Sun, 06 Jun 2010 20:48:19 -0700, Jeremy Huddleston <jeremyhu at apple.com> wrote:
>> Right.  This was the change that was merged in:
>> 
>> -    if(!dixRequestPrivate(driGCKey, sizeof(DRIGCRec)))
>> +    if(!dixRegisterPrivateKey(&driGCKeyRec, PRIVATE_GC, sizeof(DRIGCRec)))
>>        return FALSE;
>> 
>> -    if(!dixRequestPrivate(driWrapScreenKey, sizeof(DRIWrapScreenRec)))
>> +    if(!dixRegisterPrivateKey(&driWrapScreenKeyRec, PRIVATE_WINDOW, sizeof(DRIWrapScreenRec)))
>>        return FALSE;
>> 
>> What is the correct way to do this with the new API, or was this
>> always broken and we never knew it?
> 
> The change had a bug in it (PRIVATE_WINDOW instead of PRIVATE_SCREEN),
> but the original code also had a bug:
> 
>    if(!dixRegisterPrivateKey(&driWrapScreenKeyRec, PRIVATE_WINDOW, sizeof(DRIWrapScreenRec)))
> 	return FALSE;
> 
>    pScreenPriv = malloc(sizeof(*pScreenPriv));
> 
>    ...
> 
>    dixSetPrivate(&pScreen->devPrivates, driWrapScreenKey, pScreenPriv);
> 
> This asks the private system for storage for a DRIWrapScreenRec and then
> allocates memory (for a DRIWrapScreenRec) and stores that in the private
> structure. You can do either, but not both of these operations -- either
> you manage the memory or you let the privates system manage the memory.
> 
> Because you're never freeing the memory you've allocated, I'd suggest
> changing this to:
> 
>    if(!dixRegisterPrivateKey(&driWrapScreenKeyRec, PRIVATE_SCREEN, sizeof(DRIWrapScreenRec)))
> 	return FALSE;
> 
>    pScreenPriv = dixGetPrivateAddr(&pScreen->devPrivates, &driWrapScreenKeyRec);
>    pScreenPriv->CreateGC = pScreen->CreateGC;
>    pScreen->CreateGC = DRICreateGC;
> 
> This lets the private system manage the memory for your private storage,
> which will free it at server reset time.
> 
> -- 
> keith.packard at intel.com