[Differential] [Requested Changes To] D1730: address race condition around setoperation

[graesslin (Martin Gräßlin)]

graesslin requested changes to this revision.
graesslin added a comment.
This revision now requires changes to proceed.


  Given that you don't connect to the timer at all, I think the usage of QTimer is wrong here. QElapsedTimer seems like the better choice here.
  
  The main change would be in configChanged where it would become:
  
    if (m_changedBlockTimer->isValid() && !m_changedBlockTimer->hasExpired(100)) {
        // still active
       m_changedBlockTimer->start();
    } else {
        // stop the timer
       m_changedBlockTimer->invalidate();
    }

INLINE COMMENTS

> daemon.cpp:86
>      delete m_lidClosedTimer;
> +    delete m_changeBlockTimer;
>  

why delete manually? Either pass this on construction or use a QScopedPointer.

I see that it's already done like that for the other cases, but I don't think it's a good idea to copy bad practice in new code.

REPOSITORY
  rKSCREEN KScreen

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

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

To: sebas, graesslin
Cc: plasma-devel, Plasma, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160601/30b15050/attachment-0001.html>