----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124253/#review82073 -----------------------------------------------------------
Ship it! Thanks! daemon/backends/upower/login1suspendjob.cpp (line 81) <https://git.reviewboard.kde.org/r/124253/#comment56401> If you're at it, please change this to the new connect syntax: connect(watcher, &QDBusPendingCallWatcher::finished, this, &Login1SuspendJob::sendResult); - Kai Uwe Broulik On Juli 4, 2015, 4:12 nachm., Maxim Mikityanskiy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124253/ > ----------------------------------------------------------- > > (Updated Juli 4, 2015, 4:12 nachm.) > > > Review request for kde-workspace. > > > Repository: powerdevil > > > Description > ------- > > At first, there is a memory leak when watcher is created in > Login1SuspendJob::doStart, then switch goes through default branch, and the > watcher is never deleted. > > The second bug is more serious. > > ```c++ > QDBusPendingReply<void> reply; > QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(reply, this); > connect(watcher, SIGNAL(finished(QDBusPendingCallWatcher*)), this, > SLOT(sendResult(QDBusPendingCallWatcher*))); > ``` > > The above piece of code creates default-initialized reply, then creates > watcher for this reply, and connects the signal to sendResult slot. But then, > in every case branch of switch, reply gets overwritten. Old reply gets > destroyed and new object assigns into that variable named reply. No watcher > is bound with the new reply! So what we got: our watcher will never be > deleted, and sendResult slot will never be called. > > To fix that bugs we should create watcher for the real reply, not for > default-initialized one. If no reply was created, don't create watcher, so > avoid the first leak. If the reply was created, create watcher for it, so > that sendResult will be called and watcher will be deleted from here. > > This simple patch fixes described problems. > > > Diffs > ----- > > daemon/backends/upower/login1suspendjob.cpp 8918b22 > > Diff: https://git.reviewboard.kde.org/r/124253/diff/ > > > Testing > ------- > > > Thanks, > > Maxim Mikityanskiy > >
