From 697128464b60f40c29abb6bfec6f566d5c056937 Mon Sep 17 00:00:00 2001 From: Megamouse Date: Sat, 10 Aug 2019 13:50:27 +0200 Subject: [PATCH] Qt: add some sanity checks to prevent list crashes --- rpcs3/rpcs3qt/save_manager_dialog.cpp | 64 ++++++++++++++++++++----- rpcs3/rpcs3qt/trophy_manager_dialog.cpp | 36 +++++++++++--- 2 files changed, 81 insertions(+), 19 deletions(-) diff --git a/rpcs3/rpcs3qt/save_manager_dialog.cpp b/rpcs3/rpcs3qt/save_manager_dialog.cpp index 6f694e770a..84c75174c5 100644 --- a/rpcs3/rpcs3qt/save_manager_dialog.cpp +++ b/rpcs3/rpcs3qt/save_manager_dialog.cpp @@ -187,9 +187,14 @@ void save_manager_dialog::Init(std::string dir) connect(m_button_delete, &QAbstractButton::clicked, this, &save_manager_dialog::OnEntriesRemove); connect(m_button_folder, &QAbstractButton::clicked, [=]() { - int idx = m_list->currentRow(); - int idx_real = m_list->item(idx, 1)->data(Qt::UserRole).toInt(); - QString path = qstr(m_dir + m_save_entries[idx_real].dirName + "/"); + const int idx = m_list->currentRow(); + QTableWidgetItem* item = m_list->item(idx, 1); + if (!item) + { + return; + } + const int idx_real = item->data(Qt::UserRole).toInt(); + const QString path = qstr(m_dir + m_save_entries[idx_real].dirName + "/"); QDesktopServices::openUrl(QUrl("file:///" + path)); }); connect(slider_icon_size, &QAbstractSlider::valueChanged, this, &save_manager_dialog::SetIconSize); @@ -197,11 +202,17 @@ void save_manager_dialog::Init(std::string dir) connect(m_list, &QTableWidget::customContextMenuRequested, this, &save_manager_dialog::ShowContextMenu); connect(m_list, &QTableWidget::cellChanged, [&](int row, int col) { - int originalIndex = m_list->item(row, 1)->data(Qt::UserRole).toInt(); - SaveDataEntry originalEntry = m_save_entries[originalIndex]; - QString originalDirName = qstr(originalEntry.dirName); + QTableWidgetItem* user_item = m_list->item(row, 1); + QTableWidgetItem* text_item = m_list->item(row, col); + if (!user_item || !text_item) + { + return; + } + const int originalIndex = user_item->data(Qt::UserRole).toInt(); + const SaveDataEntry originalEntry = m_save_entries[originalIndex]; + const QString originalDirName = qstr(originalEntry.dirName); QVariantMap currNotes = m_gui_settings->GetValue(gui::m_saveNotes).toMap(); - currNotes[originalDirName] = m_list->item(row, col)->text(); + currNotes[originalDirName] = text_item->text(); m_gui_settings->SetValue(gui::m_saveNotes, currNotes); }); connect(m_list, &QTableWidget::itemSelectionChanged, this, &save_manager_dialog::UpdateDetails); @@ -287,6 +298,10 @@ void save_manager_dialog::UpdateIcons() for (int i = 0; i < m_list->rowCount(); i++) { QTableWidgetItem* item = m_list->item(i, 0); + if (!item) + { + continue; + } QPixmap data = item->data(Qt::UserRole).value(); QPixmap icon = QPixmap(data.size() * dpr); @@ -328,7 +343,12 @@ void save_manager_dialog::OnEntryRemove() int idx = m_list->currentRow(); if (idx != -1) { - int idx_real = m_list->item(idx, 1)->data(Qt::UserRole).toInt(); + QTableWidgetItem* item = m_list->item(idx, 1); + if (!item) + { + return; + } + const int idx_real = item->data(Qt::UserRole).toInt(); if (QMessageBox::question(this, tr("Delete Confirmation"), tr("Are you sure you want to delete:\n%1?").arg(qstr(m_save_entries[idx_real].title)), QMessageBox::Yes, QMessageBox::No) == QMessageBox::Yes) { fs::remove_all(m_dir + m_save_entries[idx_real].dirName + "/"); @@ -356,7 +376,12 @@ void save_manager_dialog::OnEntriesRemove() qSort(selection.begin(), selection.end(), qGreater()); for (QModelIndex index : selection) { - int idx_real = m_list->item(index.row(), 1)->data(Qt::UserRole).toInt(); + QTableWidgetItem* item = m_list->item(index.row(), 1); + if (!item) + { + continue; + } + const int idx_real = item->data(Qt::UserRole).toInt(); fs::remove_all(m_dir + m_save_entries[idx_real].dirName + "/"); m_list->removeRow(index.row()); } @@ -389,8 +414,13 @@ void save_manager_dialog::ShowContextMenu(const QPoint &pos) connect(removeAct, &QAction::triggered, this, &save_manager_dialog::OnEntriesRemove); // entriesremove handles case of one as well connect(showDirAct, &QAction::triggered, [=]() { - int idx_real = m_list->item(idx, 1)->data(Qt::UserRole).toInt(); - QString path = qstr(m_dir + m_save_entries[idx_real].dirName + "/"); + QTableWidgetItem* item = m_list->item(idx, 1); + if (!item) + { + return; + } + const int idx_real = item->data(Qt::UserRole).toInt(); + const QString path = qstr(m_dir + m_save_entries[idx_real].dirName + "/"); QDesktopServices::openUrl(QUrl("file:///" + path)); }); @@ -438,10 +468,18 @@ void save_manager_dialog::UpdateDetails() else { const int row = m_list->currentRow(); - const int idx = m_list->item(row, 1)->data(Qt::UserRole).toInt(); + QTableWidgetItem* item = m_list->item(row, 1); + QTableWidgetItem* icon_item = m_list->item(row, 0); + + if (!item || !icon_item) + { + return; + } + + const int idx = item->data(Qt::UserRole).toInt(); const SaveDataEntry& save = m_save_entries[idx]; - m_details_icon->setPixmap(m_list->item(row, 0)->data(Qt::UserRole).value()); + m_details_icon->setPixmap(icon_item->data(Qt::UserRole).value()); m_details_title->setText(qstr(save.title)); m_details_subtitle->setText(qstr(save.subtitle)); m_details_modified->setText(tr("Last modified: %1").arg(FormatTimestamp(save.mtime))); diff --git a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp index 230d709114..89c05881aa 100644 --- a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp +++ b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp @@ -319,7 +319,12 @@ trophy_manager_dialog::trophy_manager_dialog(std::shared_ptr gui_s { return; } - m_game_combo->setCurrentText(m_game_table->item(m_game_table->selectedItems().first()->row(), GameColumns::GameName)->text()); + QTableWidgetItem* item = m_game_table->item(m_game_table->selectedItems().first()->row(), GameColumns::GameName); + if (!item) + { + return; + } + m_game_combo->setCurrentText(item->text()); }); RepaintUI(); @@ -470,6 +475,10 @@ void trophy_manager_dialog::HandleRepaintUiRequest() void trophy_manager_dialog::ResizeGameIcon(int index) { QTableWidgetItem* item = m_game_table->item(index, GameColumns::GameIcon); + if (!item) + { + return; + } const QPixmap icon = item->data(Qt::UserRole).value(); const int dpr = devicePixelRatio(); @@ -512,7 +521,13 @@ void trophy_manager_dialog::ResizeTrophyIcons() for (int i = 0; i < m_trophy_table->rowCount(); ++i) { - const int trophy_id = m_trophy_table->item(i, TrophyColumns::Id)->text().toInt(); + QTableWidgetItem* item = m_trophy_table->item(i, TrophyColumns::Id); + QTableWidgetItem* icon_item = m_trophy_table->item(i, TrophyColumns::Icon); + if (!item || !icon_item) + { + continue; + } + const int trophy_id = item->text().toInt(); const QPixmap icon = m_trophies_db[db_pos]->trophy_images[trophy_id]; QPixmap new_icon = QPixmap(icon.size() * dpr); @@ -527,7 +542,7 @@ void trophy_manager_dialog::ResizeTrophyIcons() } const QPixmap scaled = new_icon.scaledToHeight(new_height, Qt::SmoothTransformation); - m_trophy_table->item(i, TrophyColumns::Icon)->setData(Qt::DecorationRole, scaled); + icon_item->setData(Qt::DecorationRole, scaled); } ReadjustTrophyTable(); @@ -542,11 +557,20 @@ void trophy_manager_dialog::ApplyFilter() for (int i = 0; i < m_trophy_table->rowCount(); ++i) { - int trophy_id = m_trophy_table->item(i, TrophyColumns::Id)->text().toInt(); - QString trophy_type = m_trophy_table->item(i, TrophyColumns::Type)->text(); + QTableWidgetItem* item = m_trophy_table->item(i, TrophyColumns::Id); + QTableWidgetItem* type_item = m_trophy_table->item(i, TrophyColumns::Type); + QTableWidgetItem* icon_item = m_trophy_table->item(i, TrophyColumns::Icon); + + if (!item || !type_item || !icon_item) + { + continue; + } + + const int trophy_id = item->text().toInt(); + const QString trophy_type = type_item->text(); // I could use boolean logic and reduce this to something much shorter and also much more confusing... - bool hidden = m_trophy_table->item(i, TrophyColumns::Icon)->data(Qt::UserRole).toBool(); + bool hidden = icon_item->data(Qt::UserRole).toBool(); bool trophy_unlocked = m_trophies_db[db_pos]->trop_usr->GetTrophyUnlockState(trophy_id); bool hide = false;