On 10 April 2018 at 10:58, Michel Dänzer <mic...@daenzer.net> wrote:
> On 2018-04-10 11:47 AM, Emil Velikov wrote:
>> On 10 April 2018 at 09:28, Michel Dänzer <mic...@daenzer.net> wrote:
>>> On 2018-04-04 04:29 PM, Emil Velikov wrote:
>>>> From: Emil Velikov <emil.veli...@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.veli...@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;
>>>> + 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
>> 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.
Fair enough. I'll swap the commit with a comment one for v2.
This way, the next person will be less tempted to send the same patch.
pPriv->ptr is freed in our ::FreeScreen callback. Latter of which gets
called by xf86DeleteScreen() as the driver ::*Probe call fails.
amd-gfx mailing list