[Differential] [Accepted] D4439: KDirWatch: fix memory leak on destruction.

2017-02-06 Thread Michael Pyne
mpyne accepted this revision.
mpyne added a comment.


  The diff as proposed is just fine.  I've been bitten by `qDeleteAll` at proc 
exit before when called on objects that have complex destructors, but that's 
not the case here.
  
  It would probably be a good idea to try to streamline the code structurally 
so that we don't have to hold pointers.   But given the difficulty that it 
would entail, I think it would deserve either a separate Differential review, 
or to "upgrade" this review to focus on the structural change for its own sake, 
instead of just working on the memleak at process exit.

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4439

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, aacid, mpyne
Cc: markg, #frameworks


[Differential] [Accepted] D4439: KDirWatch: fix memory leak on destruction.

2017-02-05 Thread Albert Astals Cid
aacid accepted this revision.
aacid added a comment.
This revision is now accepted and ready to land.


  Looks good to me, we could also go the crazy way and hold the data in 
m_clients instead of holding the ptr to the data
  http://paste.ubuntu.com/23933091/
  
  But I'm pretty sure i did some porting mistake in there :D

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4439

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, mpyne, aacid
Cc: #frameworks