Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-91253

QConcatenateTablesProxyModel crushes being sourceModel in QSortFilterProxyModel

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: 5.15.2
    • Fix Version/s: 5.15.4
    • Component/s: Core: Item Models
    • Labels:
      None
    • Platform/s:
      All

      Description

      TLDR:

      Following assertion has to be removed from QConcatenateTablesProxyModel::index method:

      // qconcatenatetablesproxymodel.cpp:292
      Q_ASSERT(hasIndex(row, column, parent));
      

      Digging into problem

      Sample code:

      QSortFilterProxyModel proxy;
      QConcatenateTablesProxyModel concat;
      proxy.setSourceModel(&concat);
      

      This example crushes when we try to remove the last sourceModel from concat. When there're no columns left in the concat, proxy updates it's information and calls this slot:

      // qsortfilterproxymodel.cpp:1785
      void QSortFilterProxyModelPrivate::_q_sourceColumnsRemoved(
      // ...
          proxy_sort_column = q->mapFromSource(model->index(0,source_sort_column, source_parent)).column();
      }
      

      where source_sort_column = -1 (even if we set it to some reasonble value).
      But model doesn't have index(0,-1), so it crashes at the mentioned assertion.

      At the same moment, this example is fully functional running in release mode, because the assertion (being removed in the release mode) follows this simple condition:

      // qconcatenatetablesproxymodel.cpp:292
      Q_ASSERT(hasIndex(row, column, parent));
      if (!hasIndex(row, column, parent))
          return QModelIndex();
      

      Is it correct to remove Q_ASSERT?

      Compare QAbstractItemModel::index implementations:

      QConcatenateTablesProxyModel QAbstractListModel
      QModelIndex QConcatenateTablesProxyModel::index(int row, int column, const QModelIndex &parent) const
      {
          Q_D(const QConcatenateTablesProxyModel);
          Q_ASSERT(hasIndex(row, column, parent));
          if (!hasIndex(row, column, parent))
              return QModelIndex();
          Q_ASSERT(checkIndex(parent, QAbstractItemModel::CheckIndexOption::ParentIsInvalid)); // flat model
          const auto result = d->sourceModelForRow(row);
          Q_ASSERT(result.sourceModel);
          return mapFromSource(result.sourceModel->index(result.sourceRow, column));
      }
      
      QModelIndex QAbstractListModel::index(int row, int column, const QModelIndex &parent) const
      {
          return hasIndex(row, column, parent) ? createIndex(row, column) : QModelIndex();
      }

      QAbstractListModel doesn't have any assertions on hasIndex, so should not QConcatenateTablesProxyModel.

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            dfaure_kdab David Faure
            Reporter:
            sirotin Igor Sirotin
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Gerrit Reviews