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

[Mark Gaiser]

markg added a comment.


  In https://phabricator.kde.org/D4439#83310, @aacid wrote:
  
  > In https://phabricator.kde.org/D4439#83166, @dfaure wrote:
  >
  > > It's not crazy, but
  > >
  > > - then it should use QVector instead of QList (Client is a "big" struct, bigger than a pointer)
  >
  >
  > The problem with QVector is that it doesn't have erase(iterator)  built in like QList has.
  >
  > > - I would be worried about copies happening unexpectedly (can this code compile with forbidden copy ctor for Client? I guess not as is due to insertion into the vector.... but maybe std::move can be used there, or simply setting the members directly onto a ref for vector[i]).
  >
  > Without the copy constructor there's quite a lot of things that don't work. OTOH all the data in Client is basiclaly POD, but i guess at some point it could be "a lot of copying", if you think it's worth it i can investigate some "less Q and more C++11-y stuff" and see if std::move or something works
  
  
  std::move and std::vector will work :)
  You "just" have to implement the move semantics for the Client class.
  Follow this: http://en.cppreference.com/w/cpp/language/move_assignment
  Look closely at the example and in there at "struct A" at these two snippets: (first one is the move constructor, second one is the move assignment operator)
  
    A(A&& o) : s(std::move(o.s)) { }
  
  
  
    A& operator=(A&& other)
  
  Also, don't forget to explicitly delete the copy operations (in the public section of your class):
  A(A const &) = delete;
  void operator=(A const &x) = delete;
  
  Good luck :)

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: markg, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170205/0ca1637e/attachment.html>