[PATCH xserver 1/4] animcur: Use fixed-size screen private

[Aaron Plattner]

On 11/12/2017 04:35 PM, Robert Morell wrote:
> On Mon, Nov 06, 2017 at 03:19:51PM -0500, Adam Jackson wrote:
>> Signed-off-by: Adam Jackson <ajax at redhat.com>
>> ---
>>   render/animcur.c | 15 ++++-----------
>>   1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/render/animcur.c b/render/animcur.c
>> index 52e6b8b79f..0707fe7271 100644
>> --- a/render/animcur.c
>> +++ b/render/animcur.c
>> @@ -77,12 +77,9 @@ static CursorBits animCursorBits = {
>>   
>>   static DevPrivateKeyRec AnimCurScreenPrivateKeyRec;
>>   
>> -#define AnimCurScreenPrivateKey (&AnimCurScreenPrivateKeyRec)
>> -
>>   #define IsAnimCur(c)	    ((c) && ((c)->bits == &animCursorBits))
>>   #define GetAnimCur(c)	    ((AnimCurPtr) ((((char *)(c) + CURSOR_REC_SIZE))))
>> -#define GetAnimCurScreen(s) ((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, AnimCurScreenPrivateKey))
>> -#define SetAnimCurScreen(s,p) dixSetPrivate(&(s)->devPrivates, AnimCurScreenPrivateKey, p)
>> +#define GetAnimCurScreen(s) ((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, &AnimCurScreenPrivateKeyRec))
>>   
>>   #define Wrap(as,s,elt,func) (((as)->elt = (s)->elt), (s)->elt = func)
>>   #define Unwrap(as,s,elt)    ((s)->elt = (as)->elt)
>> @@ -101,7 +98,6 @@ AnimCurCloseScreen(ScreenPtr pScreen)
>>       Unwrap(as, pScreen, RealizeCursor);
>>       Unwrap(as, pScreen, UnrealizeCursor);
>>       Unwrap(as, pScreen, RecolorCursor);
>> -    SetAnimCurScreen(pScreen, 0);
>>       ret = (*pScreen->CloseScreen) (pScreen);
>>       free(as);
> 
> The free() above should be removed as well, otherwise it causes a
> double-free on screen teardown.
> 
> Other than that,
> 
> Reviewed-by: Robert Morell <rmorell at nvidia.com>
> Tested-by: Robert Morell <rmorell at nvidia.com>
> 
> for the series.

At least two users have hit this now:
https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230799/#5230799

Adam, can this series be merged?

-- Aaron

> Thanks,
> Robert
> 
>>       return ret;
>> @@ -308,15 +304,13 @@ AnimCurInit(ScreenPtr pScreen)
>>   {
>>       AnimCurScreenPtr as;
>>   
>> -    if (!dixRegisterPrivateKey(&AnimCurScreenPrivateKeyRec, PRIVATE_SCREEN, 0))
>> +    if (!dixRegisterPrivateKey(&AnimCurScreenPrivateKeyRec, PRIVATE_SCREEN,
>> +                               sizeof(AnimCurScreenRec)))
>>           return FALSE;
>>   
>> -    as = (AnimCurScreenPtr) malloc(sizeof(AnimCurScreenRec));
>> -    if (!as)
>> -        return FALSE;
>> +    as = GetAnimCurScreen(pScreen);
>>       as->timer = TimerSet(NULL, TimerAbsolute, 0, AnimCurTimerNotify, pScreen);
>>       if (!as->timer) {
>> -        free(as);
>>           return FALSE;
>>       }
>>       as->timer_set = FALSE;
>> @@ -329,7 +323,6 @@ AnimCurInit(ScreenPtr pScreen)
>>       Wrap(as, pScreen, RealizeCursor, AnimCurRealizeCursor);
>>       Wrap(as, pScreen, UnrealizeCursor, AnimCurUnrealizeCursor);
>>       Wrap(as, pScreen, RecolorCursor, AnimCurRecolorCursor);
>> -    SetAnimCurScreen(pScreen, as);
>>       return TRUE;
>>   }
>>   
>> -- 
>> 2.14.2
>>