Review Request: Crash guard for KSelectionProxyModelPrivate::removeRangeFromProxy()

[Stephen Kelly]

Aaron J. Seigo wrote:

> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103421/#review8997
> -----------------------------------------------------------
> 
> Ship it!
> 
> 
> other than the style issue, this looks good :)

Apparently I can't review it after someone else has said shipit.

Please do not ship it.

What is the backtrace of the crash you are trying to prevent?

removeRangeFromProxy is only called when the selection changes, and 

KSelectionProxyModelPrivate::selectionChanged

already contains 

    if (!q->sourceModel())
        return;

So at least that part of the patch is questionable. 

Why would the m_rootIndexList be empty at that point? If there's something 
to remove then that list should not be empty.

Both should be asserted really.

The patch hides the bug, it doesn't fix it.

Thanks,

Steve.

> 
> 
> kdeui/itemviews/kselectionproxymodel.cpp
> <http://git.reviewboard.kde.org/r/103421/#comment7467>
> 
>     this should be in kdelibs style:
>     
>      if (!q->sourceModel() || m_rootIndexList.isEmpty()) {
>             return;
>     }
> 
> 
> - Aaron J. Seigo
> 
> 
> On Dec. 15, 2011, 8:54 p.m., Allen Winter wrote:
>> 
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> http://git.reviewboard.kde.org/r/103421/
>> -----------------------------------------------------------
>> 
>> (Updated Dec. 15, 2011, 8:54 p.m.)
>> 
>> 
>> Review request for kdelibs and Stephen Kelly.
>> 
>> 
>> Description
>> -------
>> 
>> Crash guard for KSelectionProxyModelPrivate::removeRangeFromProxy()
>> 
>> occassionally I am encountering asserts or crashes in this method, so
>> this little crash guard avoids those situations
>> 
>> 
>> Diffs
>> -----
>> 
>>   kdeui/itemviews/kselectionproxymodel.cpp eca2d78
>> 
>> Diff: http://git.reviewboard.kde.org/r/103421/diff/diff
>> 
>> 
>> Testing
>> -------
>> 
>> used locally for about a week and haven't seen crashes in
>> KSelectionProxyModel
>> 
>> 
>> Thanks,
>> 
>> Allen Winter
>> 
>>