dixSetPrivate regression

[Keith Packard]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100606/4df1aceb/attachment.pgp>