[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 118914: [klipper] Fix memory leaks
From: Martin_Gräßlin <mgraesslin () kde ! org>
Date: 2014-08-27 6:56:06
Message-ID: 20140827065606.11205.16032 () probe ! kde ! org
[Download RAW message or body]
--===============5257371759904457996==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118914/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 8:56 a.m.)
Status
------
This change has been discarded.
Review request for Plasma.
Repository: plasma-workspace
Description
-------
[klipper] Fix memory leaks
Klipper is leaking HistoryItems. This can happen in two cases:
* inserting when the item already exists - nobody deleted the new item
* removing items from the history
The ownership of HistoryItems are clearly passed to the History as we
can see by the fact that it deletes all items in the dtor. This means
ownership is passed to History.
For inserting it is relatively simple as most usage is just creating
a new item and inserting it. Removing is more difficult as it takes
the item and the callee might still be using it. This needs some
testing.
Diffs
-----
klipper/history.cpp 24e2ef4cf9784bcf23b8629cf4442fc90324dd8b
klipper/klipper.h 1dd520fce258e2cb147bcf6a1d83891cc56a9d73
klipper/klipper.cpp 2d6168e8517a9be23d42bb619e7c85d94d141e1b
klipper/urlgrabber.cpp 38e4919814fa633c78f3956d94b655c6c23d8fcc
Diff: https://git.reviewboard.kde.org/r/118914/diff/
Testing
-------
valgrind on a unit test I'm writing and all mem leaks fixed. Still need to properly \
test it for regressions.
Thanks,
Martin Gräßlin
--===============5257371759904457996==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/118914/">https://git.reviewboard.kde.org/r/118914/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been \
discarded.</h1> </td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Aug. 27, 2014, 8:56 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">[klipper] Fix memory leaks
Klipper is leaking HistoryItems. This can happen in two cases:
* inserting when the item already exists - nobody deleted the new item
* removing items from the history
The ownership of HistoryItems are clearly passed to the History as we
can see by the fact that it deletes all items in the dtor. This means
ownership is passed to History.
For inserting it is relatively simple as most usage is just creating
a new item and inserting it. Removing is more difficult as it takes
the item and the callee might still be using it. This needs some
testing.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">valgrind on a unit test I'm writing and all mem leaks fixed. Still \
need to properly test it for regressions.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>klipper/history.cpp <span style="color: \
grey">(24e2ef4cf9784bcf23b8629cf4442fc90324dd8b)</span></li>
<li>klipper/klipper.h <span style="color: \
grey">(1dd520fce258e2cb147bcf6a1d83891cc56a9d73)</span></li>
<li>klipper/klipper.cpp <span style="color: \
grey">(2d6168e8517a9be23d42bb619e7c85d94d141e1b)</span></li>
<li>klipper/urlgrabber.cpp <span style="color: \
grey">(38e4919814fa633c78f3956d94b655c6c23d8fcc)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118914/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============5257371759904457996==--
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic