On Sun, 06 Jun 2010 20:48:19 -0700, Jeremy Huddleston <[email protected]> 
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.

-- 
[email protected]

Attachment: pgpeLKMF6awHN.pgp
Description: PGP signature

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to