Add some checks to models

We should make sure that index() always returns an invalid model index for invalid
parameters (aka, out of bounds). This has been found with Qt's ModelTest.

I don't *think* this has any impact in Quassel, as we should never call index() with invalid
parameters anyway, but who knows. Maybe some of our crashes are related to this...
Better safe than sorry! :)

diff --git a/src/client/messagemodel.h b/src/client/messagemodel.h
index 623efa6..f4a7655 100644 (file)
--- a/src/client/messagemodel.h
+++ b/src/client/messagemodel.h
@@ -56,7 +56,7 @@ public:
 
   MessageModel(QObject *parent);
 
-  inline QModelIndex index(int row, int column, const QModelIndex &/*parent*/ = QModelIndex()) const { return createIndex(row, column); }
+  inline QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const;
   inline QModelIndex parent(const QModelIndex &) const { return QModelIndex(); }
   inline int rowCount(const QModelIndex &parent = QModelIndex()) const { return parent.isValid() ? 0 : messageCount(); }
   inline int columnCount(const QModelIndex &/*parent*/ = QModelIndex()) const { return 3; }
@@ -111,6 +111,14 @@ private:
   QHash<BufferId, int> _messagesWaiting;
 };
 
+// inlines
+QModelIndex MessageModel::index(int row, int column, const QModelIndex &parent) const {
+  if(row < 0 || row >= rowCount(parent) || column < 0 || column >= columnCount(parent))
+    return QModelIndex();
+
+  return createIndex(row, column);
+}
+
 // **************************************************
 //  MessageModelItem
 // **************************************************

diff --git a/src/client/networkmodel.cpp b/src/client/networkmodel.cpp
index fbf84c4..1c365a4 100644 (file)
--- a/src/client/networkmodel.cpp
+++ b/src/client/networkmodel.cpp
@@ -133,17 +133,17 @@ void NetworkItem::attachNetwork(Network *network) {
   _network = network;
 
   connect(network, SIGNAL(networkNameSet(QString)),
-         this, SLOT(setNetworkName(QString)));
+          this, SLOT(setNetworkName(QString)));
   connect(network, SIGNAL(currentServerSet(QString)),
-         this, SLOT(setCurrentServer(QString)));
+          this, SLOT(setCurrentServer(QString)));
   connect(network, SIGNAL(ircChannelAdded(IrcChannel *)),
-         this, SLOT(attachIrcChannel(IrcChannel *)));
+          this, SLOT(attachIrcChannel(IrcChannel *)));
   connect(network, SIGNAL(ircUserAdded(IrcUser *)),
-         this, SLOT(attachIrcUser(IrcUser *)));
+          this, SLOT(attachIrcUser(IrcUser *)));
   connect(network, SIGNAL(connectedSet(bool)),
-         this, SIGNAL(networkDataChanged()));
+          this, SIGNAL(networkDataChanged()));
   connect(network, SIGNAL(destroyed()),
-         this, SIGNAL(networkDataChanged()));
+          this, SIGNAL(networkDataChanged()));
 
   emit networkDataChanged();
 }
@@ -252,7 +252,7 @@ void BufferItem::updateActivityLevel(const Message &msg) {
     stateChanged = true;
     _firstUnreadMsgId = msg.msgId();
   }
-     
+
   BufferInfo::ActivityLevel oldLevel = activityLevel();
 
   _activity |= BufferInfo::OtherActivity;
@@ -379,10 +379,10 @@ bool QueryBufferItem::setData(int column, const QVariant &value, int role) {
     {
       QString newName = value.toString();
       if(!newName.isEmpty()) {
-       Client::renameBuffer(bufferId(), newName);
-       return true;
+        Client::renameBuffer(bufferId(), newName);
+        return true;
       } else {
-       return false;
+        return false;
       }
     }
     break;
@@ -494,7 +494,7 @@ QString ChannelBufferItem::toolTip(int column) const {
       QString _topic = topic();
       if(_topic != "") {
         _topic = stripFormatCodes(_topic);
-       _topic = Qt::escape(_topic);
+        _topic = Qt::escape(_topic);
         toolTip.append(QString("<font size='-2'>&nbsp;</font>"));
         toolTip.append(tr("<b>Topic:</b> %1").arg(_topic));
       }
@@ -512,19 +512,19 @@ void ChannelBufferItem::attachIrcChannel(IrcChannel *ircChannel) {
   _ircChannel = ircChannel;
 
   connect(ircChannel, SIGNAL(topicSet(QString)),
-         this, SLOT(setTopic(QString)));
+          this, SLOT(setTopic(QString)));
   connect(ircChannel, SIGNAL(ircUsersJoined(QList<IrcUser *>)),
-         this, SLOT(join(QList<IrcUser *>)));
+          this, SLOT(join(QList<IrcUser *>)));
   connect(ircChannel, SIGNAL(ircUserParted(IrcUser *)),
-         this, SLOT(part(IrcUser *)));
+          this, SLOT(part(IrcUser *)));
   connect(ircChannel, SIGNAL(parted()),
-         this, SLOT(ircChannelParted()));
+          this, SLOT(ircChannelParted()));
   connect(ircChannel, SIGNAL(ircUserModesSet(IrcUser *, QString)),
-         this, SLOT(userModeChanged(IrcUser *)));
+          this, SLOT(userModeChanged(IrcUser *)));
   connect(ircChannel, SIGNAL(ircUserModeAdded(IrcUser *, QString)),
-         this, SLOT(userModeChanged(IrcUser *)));
+          this, SLOT(userModeChanged(IrcUser *)));
   connect(ircChannel, SIGNAL(ircUserModeRemoved(IrcUser *, QString)),
-         this, SLOT(userModeChanged(IrcUser *)));
+          this, SLOT(userModeChanged(IrcUser *)));
 
   if(!ircChannel->ircUsers().isEmpty())
     join(ircChannel->ircUsers());
@@ -612,7 +612,7 @@ void ChannelBufferItem::removeUserFromCategory(IrcUser *ircUser) {
     categoryItem = qobject_cast<UserCategoryItem *>(child(i));
     if(categoryItem->removeUser(ircUser)) {
       if(categoryItem->childCount() == 0)
-       removeChild(i);
+        removeChild(i);
       break;
     }
   }
@@ -814,9 +814,9 @@ NetworkModel::NetworkModel(QObject *parent)
   : TreeModel(NetworkModel::defaultHeader(), parent)
 {
   connect(this, SIGNAL(rowsInserted(const QModelIndex &, int, int)),
-         this, SLOT(checkForNewBuffers(const QModelIndex &, int, int)));
+          this, SLOT(checkForNewBuffers(const QModelIndex &, int, int)));
   connect(this, SIGNAL(rowsAboutToBeRemoved(const QModelIndex &, int, int)),
-         this, SLOT(checkForRemovedBuffers(const QModelIndex &, int, int)));
+          this, SLOT(checkForRemovedBuffers(const QModelIndex &, int, int)));
 
   BufferSettings defaultSettings;
   defaultSettings.notify("UserNoticesTarget", this, SLOT(messageRedirectionSettingsChanged()));
@@ -1013,10 +1013,10 @@ void NetworkModel::updateBufferActivity(Message &msg) {
     if(bufferType(msg.bufferId()) != BufferInfo::ChannelBuffer) {
       msg.setFlags(msg.flags() | Message::Redirected);
       if(msg.flags() & Message::ServerMsg) {
-       // server notice
-       redirectionTarget = _serverNoticesTarget;
+        // server notice
+        redirectionTarget = _serverNoticesTarget;
       } else {
-       redirectionTarget = _userNoticesTarget;
+        redirectionTarget = _userNoticesTarget;
       }
     }
     break;
@@ -1045,7 +1045,7 @@ void NetworkModel::updateBufferActivity(Message &msg) {
     if(redirectionTarget & BufferSettings::StatusBuffer) {
       const NetworkItem *netItem = findNetworkItem(msg.bufferInfo().networkId());
       if(netItem) {
-       updateBufferActivity(netItem->statusBufferItem(), msg);
+        updateBufferActivity(netItem->statusBufferItem(), msg);
       }
     }
   } else {

diff --git a/src/client/treemodel.cpp b/src/client/treemodel.cpp
index 64cc26f..000eaaa 100644 (file)
--- a/src/client/treemodel.cpp
+++ b/src/client/treemodel.cpp
@@ -301,15 +301,15 @@ TreeModel::TreeModel(const QList<QVariant> &data, QObject *parent)
 
   if(Quassel::isOptionSet("debugmodel")) {
     connect(this, SIGNAL(rowsAboutToBeInserted(const QModelIndex &, int, int)),
-           this, SLOT(debug_rowsAboutToBeInserted(const QModelIndex &, int, int)));
+            this, SLOT(debug_rowsAboutToBeInserted(const QModelIndex &, int, int)));
     connect(this, SIGNAL(rowsAboutToBeRemoved(const QModelIndex &, int, int)),
-           this, SLOT(debug_rowsAboutToBeRemoved(const QModelIndex &, int, int)));
+            this, SLOT(debug_rowsAboutToBeRemoved(const QModelIndex &, int, int)));
     connect(this, SIGNAL(rowsInserted(const QModelIndex &, int, int)),
-           this, SLOT(debug_rowsInserted(const QModelIndex &, int, int)));
+            this, SLOT(debug_rowsInserted(const QModelIndex &, int, int)));
     connect(this, SIGNAL(rowsRemoved(const QModelIndex &, int, int)),
-           this, SLOT(debug_rowsRemoved(const QModelIndex &, int, int)));
+            this, SLOT(debug_rowsRemoved(const QModelIndex &, int, int)));
     connect(this, SIGNAL(dataChanged(const QModelIndex &, const QModelIndex &)),
-           this, SLOT(debug_dataChanged(const QModelIndex &, const QModelIndex &)));
+            this, SLOT(debug_dataChanged(const QModelIndex &, const QModelIndex &)));
   }
 }
 
@@ -318,7 +318,7 @@ TreeModel::~TreeModel() {
 }
 
 QModelIndex TreeModel::index(int row, int column, const QModelIndex &parent) const {
-  if(!hasIndex(row, column, parent))
+  if(row < 0 || row >= rowCount(parent) || column < 0 || column >= columnCount(parent))
     return QModelIndex();
 
   AbstractTreeItem *parentItem;
@@ -350,7 +350,8 @@ QModelIndex TreeModel::indexByItem(AbstractTreeItem *item) const {
 
 QModelIndex TreeModel::parent(const QModelIndex &index) const {
   if(!index.isValid()) {
-    qWarning() << "TreeModel::parent(): has been asked for the rootItems Parent!";
+    // ModelTest does this
+    // qWarning() << "TreeModel::parent(): has been asked for the rootItems Parent!";
     return QModelIndex();
   }
 
@@ -442,17 +443,17 @@ void TreeModel::itemDataChanged(int column) {
 
 void TreeModel::connectItem(AbstractTreeItem *item) {
   connect(item, SIGNAL(dataChanged(int)),
-         this, SLOT(itemDataChanged(int)));
+          this, SLOT(itemDataChanged(int)));
 
   connect(item, SIGNAL(beginAppendChilds(int, int)),
-         this, SLOT(beginAppendChilds(int, int)));
+          this, SLOT(beginAppendChilds(int, int)));
   connect(item, SIGNAL(endAppendChilds()),
-         this, SLOT(endAppendChilds()));
+          this, SLOT(endAppendChilds()));
 
   connect(item, SIGNAL(beginRemoveChilds(int, int)),
-         this, SLOT(beginRemoveChilds(int, int)));
+          this, SLOT(beginRemoveChilds(int, int)));
   connect(item, SIGNAL(endRemoveChilds()),
-         this, SLOT(endRemoveChilds()));
+          this, SLOT(endRemoveChilds()));
 }
 
 void TreeModel::beginAppendChilds(int firstRow, int lastRow) {