From c41451af75520a19b050347bb9c267b69773ff0a Mon Sep 17 00:00:00 2001 From: lat9nq <22451773+lat9nq@users.noreply.github.com> Date: Wed, 2 Jun 2021 15:05:45 -0400 Subject: [PATCH] yuzu qt: Revert some usages of string_view Causes a heap-use-after free reported by AddressSanitizer. This makes use of std::filesystem::path, but due to that we have to use their string() function which may not work for all characters. --- src/yuzu/configuration/config.cpp | 7 ++++--- src/yuzu/configuration/config.h | 4 ++-- src/yuzu/configuration/configure_per_game.cpp | 11 ++++++++--- src/yuzu/configuration/configure_per_game.h | 3 ++- src/yuzu/game_list.h | 2 +- src/yuzu/main.cpp | 14 ++++++++------ src/yuzu/main.h | 4 ++-- 7 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/yuzu/configuration/config.cpp b/src/yuzu/configuration/config.cpp index 552454acf..e9d4bef60 100644 --- a/src/yuzu/configuration/config.cpp +++ b/src/yuzu/configuration/config.cpp @@ -16,7 +16,7 @@ namespace FS = Common::FS; -Config::Config(std::string_view config_name, ConfigType config_type) : type(config_type) { +Config::Config(const std::string& config_name, ConfigType config_type) : type(config_type) { global = config_type == ConfigType::GlobalConfig; Initialize(config_name); @@ -242,7 +242,7 @@ const std::array Config::default_hotkeys{{ }}; // clang-format on -void Config::Initialize(std::string_view config_name) { +void Config::Initialize(const std::string& config_name) { const auto fs_config_loc = FS::GetYuzuPath(FS::YuzuPath::ConfigDir); const auto config_file = fmt::format("{}.ini", config_name); @@ -255,7 +255,8 @@ void Config::Initialize(std::string_view config_name) { Reload(); break; case ConfigType::PerGameConfig: - qt_config_loc = FS::PathToUTF8String(fs_config_loc / "custom" / config_file); + qt_config_loc = + FS::PathToUTF8String(fs_config_loc / "custom" / FS::ToU8String(config_file)); void(FS::CreateParentDir(qt_config_loc)); qt_config = std::make_unique(QString::fromStdString(qt_config_loc), QSettings::IniFormat); diff --git a/src/yuzu/configuration/config.h b/src/yuzu/configuration/config.h index 114a2eaa7..ce3355588 100644 --- a/src/yuzu/configuration/config.h +++ b/src/yuzu/configuration/config.h @@ -22,7 +22,7 @@ public: InputProfile, }; - explicit Config(std::string_view config_name = "qt-config", + explicit Config(const std::string& config_name = "qt-config", ConfigType config_type = ConfigType::GlobalConfig); ~Config(); @@ -45,7 +45,7 @@ public: static const std::array default_hotkeys; private: - void Initialize(std::string_view config_name); + void Initialize(const std::string& config_name); void ReadValues(); void ReadPlayerValue(std::size_t player_index); diff --git a/src/yuzu/configuration/configure_per_game.cpp b/src/yuzu/configuration/configure_per_game.cpp index 7dfcf150c..a1d434aca 100644 --- a/src/yuzu/configuration/configure_per_game.cpp +++ b/src/yuzu/configuration/configure_per_game.cpp @@ -3,10 +3,13 @@ // Refer to the license.txt file included. #include +#include #include #include #include +#include + #include #include #include @@ -18,6 +21,7 @@ #include #include +#include "common/fs/fs_util.h" #include "common/fs/path_util.h" #include "core/core.h" #include "core/file_sys/control_metadata.h" @@ -31,10 +35,11 @@ #include "yuzu/uisettings.h" #include "yuzu/util/util.h" -ConfigurePerGame::ConfigurePerGame(QWidget* parent, u64 title_id, std::string_view file_name) +ConfigurePerGame::ConfigurePerGame(QWidget* parent, u64 title_id, const std::string& file_name) : QDialog(parent), ui(std::make_unique()), title_id(title_id) { - const auto config_file_name = - title_id == 0 ? Common::FS::GetFilename(file_name) : fmt::format("{:016X}", title_id); + const auto file_path = std::filesystem::path(Common::FS::ToU8String(file_name)); + const auto config_file_name = title_id == 0 ? Common::FS::PathToUTF8String(file_path.filename()) + : fmt::format("{:016X}", title_id); game_config = std::make_unique(config_file_name, Config::ConfigType::PerGameConfig); Settings::SetConfiguringGlobal(false); diff --git a/src/yuzu/configuration/configure_per_game.h b/src/yuzu/configuration/configure_per_game.h index dc6b68763..a2d0211a3 100644 --- a/src/yuzu/configuration/configure_per_game.h +++ b/src/yuzu/configuration/configure_per_game.h @@ -28,7 +28,8 @@ class ConfigurePerGame : public QDialog { Q_OBJECT public: - explicit ConfigurePerGame(QWidget* parent, u64 title_id, std::string_view file_name); + // Cannot use std::filesystem::path due to https://bugreports.qt.io/browse/QTBUG-73263 + explicit ConfigurePerGame(QWidget* parent, u64 title_id, const std::string& file_name); ~ConfigurePerGame() override; /// Save all button configurations to settings file diff --git a/src/yuzu/game_list.h b/src/yuzu/game_list.h index 2867f6653..ab6866735 100644 --- a/src/yuzu/game_list.h +++ b/src/yuzu/game_list.h @@ -89,7 +89,7 @@ signals: void OpenTransferableShaderCacheRequested(u64 program_id); void RemoveInstalledEntryRequested(u64 program_id, InstalledEntryType type); void RemoveFileRequested(u64 program_id, GameListRemoveTarget target, - std::string_view game_path); + const std::string& game_path); void DumpRomFSRequested(u64 program_id, const std::string& game_path); void CopyTIDRequested(u64 program_id); void NavigateToGamedbEntryRequested(u64 program_id, diff --git a/src/yuzu/main.cpp b/src/yuzu/main.cpp index dd8dd3233..237e26829 100644 --- a/src/yuzu/main.cpp +++ b/src/yuzu/main.cpp @@ -1334,8 +1334,9 @@ void GMainWindow::BootGame(const QString& filename, std::size_t program_index) { if (!(loader == nullptr || loader->ReadProgramId(title_id) != Loader::ResultStatus::Success)) { // Load per game settings + const auto file_path = std::filesystem::path{filename.toStdU16String()}; const auto config_file_name = title_id == 0 - ? Common::FS::GetFilename(filename.toStdString()) + ? Common::FS::PathToUTF8String(file_path.filename()) : fmt::format("{:016X}", title_id); Config per_game_config(config_file_name, Config::ConfigType::PerGameConfig); } @@ -1799,7 +1800,7 @@ void GMainWindow::RemoveAddOnContent(u64 program_id, const QString& entry_type) } void GMainWindow::OnGameListRemoveFile(u64 program_id, GameListRemoveTarget target, - std::string_view game_path) { + const std::string& game_path) { const QString question = [this, target] { switch (target) { case GameListRemoveTarget::ShaderCache: @@ -1846,10 +1847,11 @@ void GMainWindow::RemoveTransferableShaderCache(u64 program_id) { } } -void GMainWindow::RemoveCustomConfiguration(u64 program_id, std::string_view game_path) { - const auto config_file_name = program_id == 0 - ? fmt::format("{:s}.ini", Common::FS::GetFilename(game_path)) - : fmt::format("{:016X}.ini", program_id); +void GMainWindow::RemoveCustomConfiguration(u64 program_id, const std::string& game_path) { + const auto file_path = std::filesystem::path(Common::FS::ToU8String(game_path)); + const auto config_file_name = + program_id == 0 ? Common::FS::PathToUTF8String(file_path.filename()).append(".ini") + : fmt::format("{:016X}.ini", program_id); const auto custom_config_file_path = Common::FS::GetYuzuPath(Common::FS::YuzuPath::ConfigDir) / "custom" / config_file_name; diff --git a/src/yuzu/main.h b/src/yuzu/main.h index 135681d41..490b6889f 100644 --- a/src/yuzu/main.h +++ b/src/yuzu/main.h @@ -237,7 +237,7 @@ private slots: void OnTransferableShaderCacheOpenFile(u64 program_id); void OnGameListRemoveInstalledEntry(u64 program_id, InstalledEntryType type); void OnGameListRemoveFile(u64 program_id, GameListRemoveTarget target, - std::string_view game_path); + const std::string& game_path); void OnGameListDumpRomFS(u64 program_id, const std::string& game_path); void OnGameListCopyTID(u64 program_id); void OnGameListNavigateToGamedbEntry(u64 program_id, @@ -276,7 +276,7 @@ private: void RemoveUpdateContent(u64 program_id, const QString& entry_type); void RemoveAddOnContent(u64 program_id, const QString& entry_type); void RemoveTransferableShaderCache(u64 program_id); - void RemoveCustomConfiguration(u64 program_id, std::string_view game_path); + void RemoveCustomConfiguration(u64 program_id, const std::string& game_path); std::optional SelectRomFSDumpTarget(const FileSys::ContentProvider&, u64 program_id); InstallResult InstallNSPXCI(const QString& filename); InstallResult InstallNCA(const QString& filename);