From 60afb1d1b44762868e4ac97e2a3eae106e70e3f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sun, 26 Nov 2017 17:22:37 +0100 Subject: [PATCH 1/3] WX: Fix argument parsing Manually convert each argument to a UTF-8 std::string, because the implicit conversion for wxCmdLineArgsArray to char** calls ToAscii (which is obviously undesired). Fixes https://bugs.dolphin-emu.org/issues/10274 --- Source/Core/DolphinWX/Main.cpp | 9 ++++++++- Source/Core/UICommon/CommandLineParse.cpp | 20 ++++++++++++++++---- Source/Core/UICommon/CommandLineParse.h | 4 ++++ 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Source/Core/DolphinWX/Main.cpp b/Source/Core/DolphinWX/Main.cpp index fd87aaea8e..2d79c61f1c 100644 --- a/Source/Core/DolphinWX/Main.cpp +++ b/Source/Core/DolphinWX/Main.cpp @@ -165,7 +165,14 @@ bool DolphinApp::OnInit() void DolphinApp::ParseCommandLine() { auto parser = CommandLineParse::CreateParser(CommandLineParse::ParserOptions::IncludeGUIOptions); - optparse::Values& options = CommandLineParse::ParseArguments(parser.get(), argc, argv); + + // Manually convert each argument to a UTF-8 std::string, because the implicit + // conversion of wxCmdLineArgsArray to char** calls ToAscii (which is undesired). + std::vector utf8_argv; + for (int i = 1; i < argc; ++i) + utf8_argv.emplace_back(argv[i].utf8_str()); + optparse::Values& options = CommandLineParse::ParseArguments(parser.get(), utf8_argv); + std::vector args = parser->args(); if (options.is_set("exec")) diff --git a/Source/Core/UICommon/CommandLineParse.cpp b/Source/Core/UICommon/CommandLineParse.cpp index 236cd86ba2..e8cdc6b3b6 100644 --- a/Source/Core/UICommon/CommandLineParse.cpp +++ b/Source/Core/UICommon/CommandLineParse.cpp @@ -105,17 +105,29 @@ std::unique_ptr CreateParser(ParserOptions options) return parser; } -optparse::Values& ParseArguments(optparse::OptionParser* parser, int argc, char** argv) +static void AddConfigLayer(const optparse::Values& options) { - optparse::Values& options = parser->parse_args(argc, argv); - const std::list& config_args = options.all("config"); - if (config_args.size()) + if (!config_args.empty()) { Config::AddLayer(std::make_unique( config_args, static_cast(options.get("video_backend")), static_cast(options.get("audio_emulation")))); } +} + +optparse::Values& ParseArguments(optparse::OptionParser* parser, int argc, char** argv) +{ + optparse::Values& options = parser->parse_args(argc, argv); + AddConfigLayer(options); + return options; +} + +optparse::Values& ParseArguments(optparse::OptionParser* parser, + const std::vector& arguments) +{ + optparse::Values& options = parser->parse_args(arguments); + AddConfigLayer(options); return options; } } diff --git a/Source/Core/UICommon/CommandLineParse.h b/Source/Core/UICommon/CommandLineParse.h index ffe2fc87ee..98a7da981a 100644 --- a/Source/Core/UICommon/CommandLineParse.h +++ b/Source/Core/UICommon/CommandLineParse.h @@ -3,6 +3,8 @@ // Refer to the license.txt file included. #include +#include +#include namespace optparse { @@ -20,4 +22,6 @@ enum class ParserOptions std::unique_ptr CreateParser(ParserOptions options); optparse::Values& ParseArguments(optparse::OptionParser* parser, int argc, char** argv); +optparse::Values& ParseArguments(optparse::OptionParser* parser, + const std::vector& arguments); } From 653977cec758514027a0c73937b781f60c18ced9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sun, 26 Nov 2017 18:14:43 +0100 Subject: [PATCH 2/3] UICommon: Fix unsafe usage of optparse::Values::all The const-qualified all() member method triggers undefined behaviour if the option passed to it is not set. --- Source/Core/UICommon/CommandLineParse.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/UICommon/CommandLineParse.cpp b/Source/Core/UICommon/CommandLineParse.cpp index e8cdc6b3b6..1ca4bb90e8 100644 --- a/Source/Core/UICommon/CommandLineParse.cpp +++ b/Source/Core/UICommon/CommandLineParse.cpp @@ -107,9 +107,9 @@ std::unique_ptr CreateParser(ParserOptions options) static void AddConfigLayer(const optparse::Values& options) { - const std::list& config_args = options.all("config"); - if (!config_args.empty()) + if (options.is_set_by_user("config")) { + const std::list& config_args = options.all("config"); Config::AddLayer(std::make_unique( config_args, static_cast(options.get("video_backend")), static_cast(options.get("audio_emulation")))); From 05c8d229afc3d88f4c368a648e96b049ccd2fba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sun, 26 Nov 2017 18:24:01 +0100 Subject: [PATCH 3/3] Config: Handle unknown system strings better Currently, a simple typo in the system name will trigger an assert message that complains about a "programming error". This is not user friendly and misleading. So this changes GetSystemFromName to return an std::optional, which allows for callers to check whether the system exists and handle failures better. --- Source/Core/Common/Config/Config.cpp | 5 ++--- Source/Core/Common/Config/Config.h | 3 ++- Source/Core/Core/ConfigLoaders/GameConfigLoader.cpp | 5 +++-- Source/Core/UICommon/CommandLineParse.cpp | 9 +++++++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Source/Core/Common/Config/Config.cpp b/Source/Core/Common/Config/Config.cpp index a3504f80d2..342ef8ec6d 100644 --- a/Source/Core/Common/Config/Config.cpp +++ b/Source/Core/Common/Config/Config.cpp @@ -101,15 +101,14 @@ const std::string& GetSystemName(System system) return system_to_name.at(system); } -System GetSystemFromName(const std::string& name) +std::optional GetSystemFromName(const std::string& name) { const auto system = std::find_if(system_to_name.begin(), system_to_name.end(), [&name](const auto& entry) { return entry.second == name; }); if (system != system_to_name.end()) return system->first; - _assert_msg_(COMMON, false, "Programming error! Couldn't convert '%s' to system!", name.c_str()); - return System::Main; + return {}; } const std::string& GetLayerName(LayerType layer) diff --git a/Source/Core/Common/Config/Config.h b/Source/Core/Common/Config/Config.h index 3536f5677c..14a2dcb20c 100644 --- a/Source/Core/Common/Config/Config.h +++ b/Source/Core/Common/Config/Config.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include "Common/Config/ConfigInfo.h" @@ -38,7 +39,7 @@ void Shutdown(); void ClearCurrentRunLayer(); const std::string& GetSystemName(System system); -System GetSystemFromName(const std::string& system); +std::optional GetSystemFromName(const std::string& system); const std::string& GetLayerName(LayerType layer); LayerType GetActiveLayerForConfig(const ConfigLocation&); diff --git a/Source/Core/Core/ConfigLoaders/GameConfigLoader.cpp b/Source/Core/Core/ConfigLoaders/GameConfigLoader.cpp index e6ec7c97bc..63239b903a 100644 --- a/Source/Core/Core/ConfigLoaders/GameConfigLoader.cpp +++ b/Source/Core/Core/ConfigLoaders/GameConfigLoader.cpp @@ -157,8 +157,9 @@ static ConfigLocation MapINIToRealLocation(const std::string& section, const std std::getline(buffer, config_section, '.'); fail |= buffer.fail(); - if (!fail) - return {Config::GetSystemFromName(system_str), config_section, key}; + const std::optional system = Config::GetSystemFromName(system_str); + if (!fail && system) + return {*system, config_section, key}; WARN_LOG(CORE, "Unknown game INI option in section %s: %s", section.c_str(), key.c_str()); return {Config::System::Main, "", ""}; diff --git a/Source/Core/UICommon/CommandLineParse.cpp b/Source/Core/UICommon/CommandLineParse.cpp index 1ca4bb90e8..683738479f 100644 --- a/Source/Core/UICommon/CommandLineParse.cpp +++ b/Source/Core/UICommon/CommandLineParse.cpp @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #include +#include #include #include #include @@ -40,8 +41,12 @@ public: std::getline(buffer, section, '.'); std::getline(buffer, key, '='); std::getline(buffer, value, '='); - Config::System system = Config::GetSystemFromName(system_str); - m_values.emplace_back(std::make_tuple(Config::ConfigLocation{system, section, key}, value)); + const std::optional system = Config::GetSystemFromName(system_str); + if (system) + { + m_values.emplace_back( + std::make_tuple(Config::ConfigLocation{*system, section, key}, value)); + } } }