On Wednesday, 22 February 2017, Michel Dänzer <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Michel Dänzer <<a href="javascript:;" onclick="_e(event, 'cvml', 'michel.daenzer@amd.com')">michel.daenzer@amd.com</a>><br>
<br>
Multiple calls to xf86EdidMonitorSet (which can be triggered e.g. by<br>
running xrandr) would potentially keep adding the same modes, causing<br>
the Monitor->Modes list to keep growing larger and using up more memory.<br>
<br>
Fix this by calling xf86PruneDuplicateModes after adding the modes<br>
returned by xf86DDCGetModes. This removes any newly added modes which<br>
were already in the Monitor->Modes list before, but keeps new modes<br>
which weren't yet.<br>
<br>
Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=99521" target="_blank">https://bugs.freedesktop.org/<wbr>show_bug.cgi?id=99521</a><br>
Signed-off-by: Michel Dänzer <<a href="javascript:;" onclick="_e(event, 'cvml', 'michel.daenzer@amd.com')">michel.daenzer@amd.com</a>><br>
---<br>
<br>
Xiaogang / Jorge, does this fix the memory leak you're seeing?<br>
<br>
hw/xfree86/modes/<wbr>xf86EdidModes.c | 14 ++++++--------<br>
1 file changed, 6 insertions(+), 8 deletions(-)<br>
<br>
diff --git a/hw/xfree86/modes/<wbr>xf86EdidModes.c b/hw/xfree86/modes/<wbr>xf86EdidModes.c<br>
index f0e1e974b..55833f886 100644<br>
--- a/hw/xfree86/modes/<wbr>xf86EdidModes.c<br>
+++ b/hw/xfree86/modes/<wbr>xf86EdidModes.c<br>
@@ -1198,21 +1198,19 @@ xf86EdidMonitorSet(int scrnIndex, MonPtr Monitor, xf86MonPtr DDC)<br>
if (!Monitor->nHsync || !Monitor->nVrefresh)<br>
DDCGuessRangesFromModes(<wbr>scrnIndex, Monitor, Modes);<br>
<br>
- /* look for last Mode */<br>
- Mode = Modes;<br>
-<br>
- while (Mode->next)<br>
- Mode = Mode->next;<br>
-<br>
/* add to MonPtr */<br>
if (Monitor->Modes) {<br>
Monitor->Last->next = Modes;<br>
Modes->prev = Monitor->Last;<br>
- Monitor->Last = Mode;<br>
}<br>
else {<br>
Monitor->Modes = Modes;<br>
- Monitor->Last = Mode;<br>
}<br>
+<br>
+ xf86PruneDuplicateModes(<wbr>Monitor->Modes);<br>
+</blockquote><div>Wouldn't it be better to add the mode only if it doesn't exist, or it doesn't really matter ?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ /* Update pointer to last mode */<br>
+ for (Mode = Monitor->Modes; Mode && Mode->next; Mode = Mode->next);<br>
+ Monitor->Last = Mode;</blockquote><div>Perhaps I'm just tired but this looks like this will overwrite Monitor->Last multiple times, right?</div><div><br></div><div>A simple while loop like the original code should do it.</div><div><br></div><div>-Emil</div><div>P.s. Sending from my phone - pardon the html formatting. </div>