----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113360/#review45380 -----------------------------------------------------------
Ship it! To be completely honest, the QPointer crash doesn't make sense at all and sounds more like an actual Qt bug more than a PolkitKCM bug. I don't think that would be a good solution in the big picture. That said, it's also true that the QPointer there is pretty random. Looks like the whole code needs quite a revamp. So I'm giving a Ship it to the patch because: * It fixes a leak * The QPointer was, apparently, mere overhead, as there are no situations where a null check happens or is needed at all. So I even wonder why it was there in the first place... Would be nice to know, anyways, whether you experienced that crash with Qt4 or Qt5, and if it is reproducible anywhere else. There's still no apparent justification for this to be honest. Thanks for the patch anyway. polkitactions/PolkitActionsKCM.cpp <http://git.reviewboard.kde.org/r/113360/#comment32382> Really bad, wondering why nobody ever noticed this? Another thing - it's probably clearer if we put new PolkitKde::ActionWidget(this) as an initializer for m_actionWidget in the constructor. - Dario Freddi On Dec. 8, 2013, 10:45 a.m., Ivan Shapovalov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113360/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2013, 10:45 a.m.) > > > Review request for kde-workspace, Polkit KDE Configuration and Dario Freddi. > > > Repository: polkit-kde-kcmodules-1 > > > Description > ------- > > QPointer<> crashes for me (Arch), the crash is non-debuggable (only in > Release mode) and I do not see a way how can m_actionWidget suddenly > disappear. > > > Diffs > ----- > > polkitactions/PolkitActionsKCM.h 84c6581 > polkitactions/PolkitActionsKCM.cpp a88bf65 > > Diff: http://git.reviewboard.kde.org/r/113360/diff/ > > > Testing > ------- > > - Ensured that it fixes the crash for me > - Made some test modifications and ensured that it does not crash in other > places > > > Thanks, > > Ivan Shapovalov > >
