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

[Emil Velikov]

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?
-Emil