[PATCH xserver] edid: Prune duplicates after adding modes from DDC

[Chen, Xiaogang]

> -----Original Message-----
> From: Michel Dänzer [mailto:michel at daenzer.net]
> Sent: Wednesday, February 22, 2017 2:09 AM
> To: xorg-devel at lists.x.org
> Cc: Chen, Xiaogang <Xiaogang.Chen at amd.com>;
> jorge_monteagudo at hotmail.com
> Subject: [PATCH xserver] edid: Prune duplicates after adding modes from DDC
> 
> From: Michel Dänzer <michel.daenzer at amd.com>
> 
> Multiple calls to xf86EdidMonitorSet (which can be triggered e.g. by running
> xrandr) would potentially keep adding the same modes, causing the Monitor-
> >Modes list to keep growing larger and using up more memory.
> 
> Fix this by calling xf86PruneDuplicateModes after adding the modes returned
> by xf86DDCGetModes. This removes any newly added modes which were
> already in the Monitor->Modes list before, but keeps new modes which weren't
> yet.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99521
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
> 
> Xiaogang / Jorge, does this fix the memory leak you're seeing?
> 
[XCHEN] Hi Michel: I tested your patch with same script I used before(while true; do xrandr --verbose; done), the memory leak is still there. Actually the xf86PruneDuplicateModes() is called before at xf86DDCGetModes() which is called by xf86EdidMonitorSet().

I think the point here is: why we need to have these "mode"? I do not find these "mode" got used afterword. Does anyone know the purpose that xf86DDCGetModes() generates these modes?

Jorge: did you test this patch or my patch?

Thanks
Xiaogang

>  hw/xfree86/modes/xf86EdidModes.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/xfree86/modes/xf86EdidModes.c
> b/hw/xfree86/modes/xf86EdidModes.c
> index f0e1e974b..55833f886 100644
> --- a/hw/xfree86/modes/xf86EdidModes.c
> +++ b/hw/xfree86/modes/xf86EdidModes.c
> @@ -1198,21 +1198,19 @@ xf86EdidMonitorSet(int scrnIndex, MonPtr
> Monitor, xf86MonPtr DDC)
>          if (!Monitor->nHsync || !Monitor->nVrefresh)
>              DDCGuessRangesFromModes(scrnIndex, Monitor, Modes);
> 
> -        /* look for last Mode */
> -        Mode = Modes;
> -
> -        while (Mode->next)
> -            Mode = Mode->next;
> -
>          /* add to MonPtr */
>          if (Monitor->Modes) {
>              Monitor->Last->next = Modes;
>              Modes->prev = Monitor->Last;
> -            Monitor->Last = Mode;
>          }
>          else {
>              Monitor->Modes = Modes;
> -            Monitor->Last = Mode;
>          }
> +
> +        xf86PruneDuplicateModes(Monitor->Modes);
> +
> +        /* Update pointer to last mode */
> +        for (Mode = Monitor->Modes; Mode && Mode->next; Mode = Mode-
> >next);
> +        Monitor->Last = Mode;
>      }
>  }
> --
> 2.11.0