Review Request 116666: Use a QMutableHashIterator in KItemListViewAnimation::slotFinished() for deleting an item from a hash

[Frank Reininghaus]

> On March 9, 2014, 9:49 a.m., Emmanuel Pescosta wrote:
> > Looks good!
> > 
> > Maybe there are more places were we make the same mistakes? - Smth. to keep in mind for Dolphin next ;)
> 
> Emmanuel Pescosta wrote:
>     *were = where

Yes, that could be. It's very easy to make mistakes when modifying containers while iterating through them. Maybe the best way to find such problems is to grep the source for "remove" (and possibly "erase" and other functions that modify containers) and have a close look at each occurrence.


- Frank


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


On March 9, 2014, 9:03 a.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116666/
> -----------------------------------------------------------
> 
> (Updated March 9, 2014, 9:03 a.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 331876
>     http://bugs.kde.org/show_bug.cgi?id=331876
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> KItemListViewAnimation::slotFinished() uses a QHashIterator to iterate over a QHash, and then removes an item from the hash using QHash::remove() inside the loop.
> 
> This is quite unusual - the recommended way is to use a QMutableHashIterator (or std-style iterators and then QHash::erase(it)).
> 
> This might be related to the cause of the crash.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/private/kitemlistviewanimation.cpp e347c5b 
> 
> Diff: https://git.reviewboard.kde.org/r/116666/diff/
> 
> 
> Testing
> -------
> 
> I couldn't find any regressions, but I cannot be sure that it fixes the crash because I cannot reproduce the crash.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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