Review Request 115604: Fix memory corruption on quit by canceling the active construct mode

David Narváez david.narvaez at computer.org
Wed Feb 12 16:39:45 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115604/#review49665
-----------------------------------------------------------


I initially thought I would reject this patch because, as I commented in the bug report, I would like to see this fixed by removing event loop nesting. But after taking a look at the patch, I like it a lot: it is an elegant way to deal with event loop nesting and realistically we might never have more than one level of nesting so dealing with it this way should be enough.

Yet, the fact that your implementation of KigPart::queryClose() will only return true made me wonder what's the default behavior and made me rethink the whole queryClose situation as it is now and I came up with this idea: why don't we forward the whole Kig::queryClose() to KigPart::queryClose() which will in turn forward to ReadWritePart::queryClose() and if this one returns true, then KigPart::queryClose() checks if we are in the middle of a construction (just like your patch is checking that), cancels it and then return whatever ReadWrite::queryClose() returned... makes sense? I haven't thought deeply about it but from what I see in ReadWritePart's code and what we have in Kig::queryClose I think the behavior should be the same.

Would you have time to look into this?

- David Narváez


On Feb. 9, 2014, 10:20 p.m., Jacob Welsh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115604/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2014, 10:20 p.m.)
> 
> 
> Review request for KDE Edu.
> 
> 
> Bugs: 173384
>     http://bugs.kde.org/show_bug.cgi?id=173384
> 
> 
> Repository: kig
> 
> 
> Description
> -------
> 
> Modification of three year old patch from Christoph Feck. See bug for details.
> 
> 
> Diffs
> -----
> 
>   kig/kig.cpp b3f5c8b 
>   kig/kig_part.h 7a9b151 
>   kig/kig_part.cpp 17eb0f2 
>   modes/mode.cc cb5b932 
> 
> Diff: https://git.reviewboard.kde.org/r/115604/diff/
> 
> 
> Testing
> -------
> 
> Attempted to quit the application while in a construction mode, both though close button and Quit action. With the patch, it quits without crashing. Also tested with unsaved changes; the save dialog continues to work as expected.
> 
> 
> Thanks,
> 
> Jacob Welsh
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140212/2772510a/attachment.html>


More information about the kde-edu mailing list