Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/#review79323 --- src/kidletime.cpp (lines 222 - 223) https://git.reviewboard.kde.org/r/123417/#comment54155 that would cause a crash. It will set the poller to null and afterwards unreference the nullptr. - Martin Gräßlin On April 22, 2015, 3:18 a.m., Nerdopolis Turfwalker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 3:18 a.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 7:13 a.m.) Review request for KDE Frameworks. Changes --- Try to properly fix the isAvailable crash Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs (updated) - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/#review79327 --- Ship it! Ship It! - Martin Gräßlin On April 22, 2015, 9:13 a.m., Nerdopolis Turfwalker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 9:13 a.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 7:27 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit d6133737bd42b7d9d4f470fba5aa794cc9257748 by Martin Gräßlin on behalf of Nerdopolis Turfwalker to branch master. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 1:18 a.m.) Review request for KDE Frameworks. Changes --- Simplify and fix the patch to protect X11 calls. Remove an uneeded test, don't set poller=0 many times. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs (updated) - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
On April 20, 2015, 5:48 a.m., Martin Gräßlin wrote: src/kidletime.cpp, lines 193-194 https://git.reviewboard.kde.org/r/123417/diff/1/?file=362046#file362046line193 suggestion: merge the two ifs with an . If isPlatformX11 is false, the other condition isn't checked, so it's save I actually realized I had to restructure this part as I had it wrong, and preventing it from using XScreensaverBasedPoller. Also setting poller=0 caused a crash in !poller-isAvailable(), (I don't know how I missed it the first time), So I merged the if statement that checks if it's NULL instead. First it checks if it's NULL, then it checks if it's availible. I retested, and plasmashell did not crash - Nerdopolis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/#review79237 --- On April 22, 2015, 1:18 a.m., Nerdopolis Turfwalker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 1:18 a.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/#review79237 --- Approach looks correct to me, but can be simplified (see below). A general comment: please watch the coding style, e.g. we use four spaces for indentation. src/kidletime.cpp (lines 193 - 194) https://git.reviewboard.kde.org/r/123417/#comment54137 suggestion: merge the two ifs with an . If isPlatformX11 is false, the other condition isn't checked, so it's save src/kidletime.cpp (line 240) https://git.reviewboard.kde.org/r/123417/#comment54138 I don't think that this check is needed. Trying to cast should not matter. - Martin Gräßlin On April 18, 2015, 7:19 p.m., Nerdopolis Turfwalker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 18, 2015, 7:19 p.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 18, 2015, 5:19 p.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
