D7139: Fix QSortFilterProxyModelPrivate::updateChildrenMapping crash in libtaskmanager

David Edmundson noreply at phabricator.kde.org
Fri Aug 4 21:18:42 UTC 2017


davidedmundson added a comment.


  Looks good. 1 tiny change needed I think.
  
  Also some code comments on the QVector would be useful for future people.

INLINE COMMENTS

> taskgroupingproxymodel.cpp:41
>  
> -    QVector<QVector<int>> rowMap;
> +    QVector<QVector<int> *> rowMap;
>  

Cleanup on destruction.

(which sounds like the name of a new Megadeth single)

> taskgroupingproxymodel.cpp:465
>  
> -    rowMap[row].resize(1);
> -
> -    // index() encodes parent row numbers in indices returned for group
> -    // members. endRemoveRows() is not aware of this scheme, and so won't
> -    // invalidate persistent indices for the members of the group we're
> -    // dissolving. We're now going to do it ourselves.
> -    foreach (const QModelIndex &idx, q->persistentIndexList()) {
> -        if (idx.internalId() == ((uint)row + 1)) {
> -            q->changePersistentIndex(idx, QModelIndex());
> -        }
> -    }
> +    rowMap[row]->resize(1);
>  

(I know this existing code)

why 1?
shouldn't this should be currentSize + extraChildren.count()

> taskgroupingproxymodel.cpp:521
> +            return index(parentRow, 0);
> +        }
> +    }

I think we should assert after this if.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7139

To: hein, #plasma, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170804/4edcb5eb/attachment.html>


More information about the Plasma-devel mailing list