[PATCH xf86-video-amdgpu 11/19] Don't leak a AMDGPUEntRec instance if amdgpu_device_setup fails

[Michel Dänzer]

On 2018-04-10 11:47 AM, Emil Velikov wrote:
> On 10 April 2018 at 09:28, Michel Dänzer <michel at daenzer.net> wrote:
>> On 2018-04-04 04:29 PM, Emil Velikov wrote:
>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>
>>> Seems like we've been leaking this for years. It became more obvious
>>> with the recent refactoring.
>>>
>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>> ---
>>>  src/amdgpu_probe.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
>>> index 537d44c..588891c 100644
>>> --- a/src/amdgpu_probe.c
>>> +++ b/src/amdgpu_probe.c
>>> @@ -243,6 +243,8 @@ amdgpu_probe(ScrnInfoPtr pScrn, int entity_num,
>>>       return TRUE;
>>>
>>>  error:
>>> +     free(pPriv->ptr);
>>> +     pPriv->ptr = NULL;
>>>       return FALSE;
>>>  }
>>>
>>>
>>
>> valgrind doesn't report a leak if I force this error path; presumably
>> Xorg frees the private after returning FALSE here.
>>
> Just double-checked and Xorg does not know anything about ptr. The
> only one who clears it up is AMDGPUFreeScreen_KMS.
> 
> The magic (for this and the other 'leak') seems to be happening in
> xf86platformAddDevice. Namely:
>  - ::platformProbe is called via doPlatformProbe
>  - the driver explicitly calls xf86AllocateScreen, yet fails later on
>  - back in Xorg, the "if (old_screens == xf86NumGPUDrivers)" is false
>  - ::PreInit fails, ::configured is false
>  - xf86DeleteScreen() gets called, which dives into ::FreeScreen (aka
> AMDGPUFreeScreen_KMS)
> 
> Eventually, I could unwrap all that although it makes sense to keep
> things simpler. As effectively done by the patch.
> 
> I believe you'll agree?

I'm afraid not. There's no leak because it's getting cleaned up as
designed, so there's no need for this change.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer