Review Request 111919: Fix possible crash when disabling grouping

[Emmanuel Pescosta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111919/#review37305
-----------------------------------------------------------

Ship it!


I can't reproduce it either. Tested with 123 items grouped in 25 groups.

> The solution is to use a QHashIterator. This iterator creates an internal copy of the container which is not modified when elements are being removed.
This makes sense, but can you please add this comment to the modified code? So that everybody knows, why we use a QHashIterator and not a QMutableHashIterator here. Thanks.


- Emmanuel Pescosta


On Aug. 6, 2013, 9:26 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111919/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 9:26 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> Sometimes when enabling and then disabling grouping, I get a crash.
> 
> I think the problem is that KItemListView::slotGroupedSortingChanged(bool current) uses a QMutableHashIterator to iterate through the groups while they are being removed. The problem is that they are not removed using the iterator, but by another function inside the loop. The iterator can thus become invalid, and iterating further can cause all sorts of trouble if we are unlucky.
> 
> The solution is to use a QHashIterator. This iterator creates an internal copy of the container which is not modified when elements are being removed.
> 
> 
> This addresses bug 323248.
>     http://bugs.kde.org/show_bug.cgi?id=323248
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistview.cpp 0c3183c 
> 
> Diff: http://git.reviewboard.kde.org/r/111919/diff/
> 
> 
> Testing
> -------
> 
> Cannot reproduce the crash any more.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130807/bcd6962a/attachment.htm>