From: Manuel Nickschas Date: Fri, 25 Dec 2009 13:41:46 +0000 (+0100) Subject: Add some checks to models X-Git-Tag: 0.6-beta1~104 X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=42406861fbe570ed1fb45e9da39ff9a0de73c284 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 623efa66..f4a76555 100644 --- 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 _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 fbf84c4f..1c365a44 100644 --- 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(" ")); toolTip.append(tr("Topic: %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)), - this, SLOT(join(QList))); + this, SLOT(join(QList))); 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(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 64cc26fe..000eaaa2 100644 --- a/src/client/treemodel.cpp +++ b/src/client/treemodel.cpp @@ -301,15 +301,15 @@ TreeModel::TreeModel(const QList &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) {