----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118914/#review60888 -----------------------------------------------------------
klipper/history.cpp <https://git.reviewboard.kde.org/r/118914/#comment42410> You have a potential crash here, the caller now has a dangling pointer. Klipper::applyClipChanges cals this and returns the inserted pointer. This is called from Klipper::checkClipData (klipper.cpp:706) and the return value is used (line 712) Option 1: you make insert have a HistoryItem return value, for duplicates you return the original Option 2: We make history item implicitly shared and stop passing pointers all over the place - 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
