Review Request 123417: Prevent plasmashell from crashing on wayland

Martin Gräßlin mgraesslin at kde.org
Mon Apr 20 05:48:57 UTC 2015


-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150420/8de84b73/attachment.html>


More information about the Kde-frameworks-devel mailing list