----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127166/#review92707 -----------------------------------------------------------
nice refactoring. The code looks much cleaner! src/klauncher/klauncher.h (line 274) <https://git.reviewboard.kde.org/r/127166/#comment63181> const QByteArray & - Martin Gräßlin On Feb. 24, 2016, 6:19 a.m., Xuetian Weng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127166/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2016, 6:19 a.m.) > > > Review request for KDE Frameworks, David Faure and Martin Gräßlin. > > > Repository: kinit > > > Description > ------- > > The ported klauncher at least has two bugs: > 1. IMHO, it should compare the cached display string with == instead of != > (though != has been there since the old kdelibs, but it just doesn't look > correct to me, what's the point of assign dpy = mCached_dpy when dpy_str != > XDisplayString(dpy)? And what's the point use == in one place and != in other > two places? ). > 2. it might free the cached connection without setting mCached.conn back to > nullptr, which could lead to double free or freeze when system logout. > > This patch refactor the code a little bit with a helper function to update > the cached connection instead of duplicate the same logic everywhere. > getXCBConnection() will make sure the returned connection is error-free, > reuse the cached connection if possible, and update the cached connection if > needed. > > > Diffs > ----- > > src/klauncher/klauncher.h 31bfaaa > src/klauncher/klauncher.cpp 7ea9da9 > > Diff: https://git.reviewboard.kde.org/r/127166/diff/ > > > Testing > ------- > > Compiles, startup notify works. > > > Thanks, > > Xuetian Weng > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
