Review Request 121436: grepview: fix two crashes
Milian Wolff
mail at milianw.de
Wed Dec 10 21:39:42 UTC 2014
> On Dec. 10, 2014, 4:21 p.m., Milian Wolff wrote:
> > plugins/grepview/grepdialog.cpp, line 468
> > <https://git.reviewboard.kde.org/r/121436/diff/1/?file=332561#file332561line468>
> >
> > instead, please use the newer connect (afaik added in Qt 5.3):
> >
> > connect(toolView, &GrepOutputView::outputViewIsClosed,
> > job, [] { job->kill(); });
> >
> > Also, couldn't it be rewritten to:
> >
> > connect(toolView, &GrepOutputView::outputViewIsClosed,
> > job, &GrepJob::kill);
> >
> > Maybe some casts are required?
>
> Aleix Pol Gonzalez wrote:
> Agreed, second option should work and looks quite elegant.
>
> Nicolai Hähnle wrote:
> KJob::kill takes one argument with a default value, which -- as far as I understand it -- is why the suggestion (which I like and actually tried first) did not compile. If there's a way to convince the Qt magic to call kill anyway with the default value, that would be nice indeed.
ah yes, then please use my first option and get rid of the QPointer again
- Milian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121436/#review71725
-----------------------------------------------------------
On Dec. 10, 2014, 3:22 p.m., Nicolai Hähnle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121436/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2014, 3:22 p.m.)
>
>
> Review request for KDevelop.
>
>
> Repository: kdevplatform
>
>
> Description
> -------
>
> The first crash is a crash-on-exit, when the kill() method of a no longer existing job is called. Guard againsts this crash by using a QPointer.
>
> The second crash happens when GrepOutputDelegate::sizeHint() is called while using the combo-box of the grep output view to switch to previous search results. Simply change the code to be more defensive.
>
>
> Diffs
> -----
>
> plugins/grepview/grepdialog.cpp bb00e47
> plugins/grepview/grepoutputdelegate.cpp bcd911b
>
> Diff: https://git.reviewboard.kde.org/r/121436/diff/
>
>
> Testing
> -------
>
> manual testing / daily use
>
>
> Thanks,
>
> Nicolai Hähnle
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20141210/61e86b0b/attachment.html>
More information about the KDevelop-devel
mailing list