> On Oct. 29, 2015, 6:39 p.m., Aroonav Mishra wrote: > > In line 160-164, I will suggest you to not completely remove those > > lines(Memory leaks maybe?) but instead revert it to the way it was before I > > edited it(I don't remember why I changed it). That is, put > > d->worker->deleteLater(); there. This shold work correctly(I haven't tested > > it) because (d->worker) is an object derived from QObject. > > > > We will have a problem if we do this in line 234-235 as ThreadWeaver::Job > > isn't a QObject. But it is important to delete that Job object to prevent a > > memory leak here. The done signal indicates that the Job object isn't going > > to be used anymore and which means that, that object should be deleted.
In line 234-235 ThreadWeaver::JobPointer is a qsharedpointer ,so we don't have to delete it manually - rishabh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125872/#review87688 ----------------------------------------------------------- On Oct. 30, 2015, 5:56 a.m., rishabh gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125872/ > ----------------------------------------------------------- > > (Updated Oct. 30, 2015, 5:56 a.m.) > > > Review request for Amarok, Stefan Derkits and Olivier Churlaud. > > > Repository: amarok > > > Description > ------- > > removed the code that was deleting the threadeweaver::job .it's a > qsharedpointer so it shouldn't be deleted manaully > > > Diffs > ----- > > src/core-impl/collections/db/sql/SqlQueryMaker.cpp c73a551 > > Diff: https://git.reviewboard.kde.org/r/125872/diff/ > > > Testing > ------- > > builds 100% > testing done > > > Thanks, > > rishabh gupta > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel