Review Request 118914: [klipper] Fix memory leaks

[David Edmundson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118914/#review60896
-----------------------------------------------------------

Ship it!



klipper/urlgrabber.cpp
<https://git.reviewboard.kde.org/r/118914/#comment42414>

    Just make the function not const?


can you null m_historyItem in clipcommandprocess.cpp:68.
It's not a problem right now, but it's still not good to leave things dangling.

Looks like it'll work, but I think it's rather fragile for when someone tries to change it in the future. insert() should't delete arguments without telling you.

If you promise to change it to implicitly shared for 5.x. Ship it.

- David Edmundson


On June 24, 2014, 10:29 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118914/
> -----------------------------------------------------------
> 
> (Updated June 24, 2014, 10:29 a.m.)
> 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140624/17e6598e/attachment.html>