From e09f2e49a63ad667513ed3800153e5f3e6722f82 Mon Sep 17 00:00:00 2001 From: Vas Crabb Date: Sat, 23 Nov 2019 11:30:07 +1100 Subject: util/options.cpp: fix locale issues and a const correctness issue --- src/emu/emuopts.cpp | 7 +- src/frontend/mame/ui/moptions.cpp | 2 +- src/lib/util/options.cpp | 203 ++++++++++++++++++++++++++------------ src/lib/util/options.h | 55 ++++++----- 4 files changed, 174 insertions(+), 93 deletions(-) diff --git a/src/emu/emuopts.cpp b/src/emu/emuopts.cpp index e3de65b14bd..c42fcef8482 100644 --- a/src/emu/emuopts.cpp +++ b/src/emu/emuopts.cpp @@ -237,7 +237,7 @@ namespace { } - virtual const char *value() const override + virtual const char *value() const noexcept override { // This is returning an empty string instead of nullptr to signify that // specifying the value is a meaningful operation. The option types that @@ -287,7 +287,7 @@ namespace { } - virtual const char *value() const override + virtual const char *value() const noexcept override { const char *result = nullptr; if (m_host.specified()) @@ -298,6 +298,7 @@ namespace // happen in practice // // In reality, I want to really return std::optional here + // FIXME: the std::string assignment can throw exceptions, and returning std::optional also isn't safe in noexcept m_temp = m_host.specified_value(); result = m_temp.c_str(); } @@ -325,7 +326,7 @@ namespace { } - virtual const char *value() const override + virtual const char *value() const noexcept override { return m_host.value().c_str(); } diff --git a/src/frontend/mame/ui/moptions.cpp b/src/frontend/mame/ui/moptions.cpp index 1b4779ddaf0..b4da221e18e 100644 --- a/src/frontend/mame/ui/moptions.cpp +++ b/src/frontend/mame/ui/moptions.cpp @@ -96,7 +96,7 @@ ui_options::ui_options() : core_options() rgb_t ui_options::rgb_value(const char *option) const { // find the entry - core_options::entry::shared_ptr entry = get_entry(option); + core_options::entry::shared_const_ptr entry = get_entry(option); // look up the value, and sanity check the result const char *value = entry->value(); diff --git a/src/lib/util/options.cpp b/src/lib/util/options.cpp index cffb7493fe3..65aac4a8883 100644 --- a/src/lib/util/options.cpp +++ b/src/lib/util/options.cpp @@ -8,14 +8,18 @@ ***************************************************************************/ -#include -#include -#include -#include #include "options.h" + #include "corestr.h" + +#include #include +#include +#include +#include +#include + const int core_options::MAX_UNADORNED_OPTIONS; @@ -134,7 +138,7 @@ core_options::entry::~entry() // entry::value //------------------------------------------------- -const char *core_options::entry::value() const +const char *core_options::entry::value() const noexcept { // returning 'nullptr' from here signifies a value entry that is essentially "write only" // and cannot be meaningfully persisted (e.g. - a command or the software name) @@ -181,43 +185,82 @@ void core_options::entry::set_default_value(std::string &&newvalue) void core_options::entry::validate(const std::string &data) { - float fval; - int ival; + std::istringstream str(data); + str.imbue(std::locale::classic()); switch (type()) { case option_type::BOOLEAN: - // booleans must be 0 or 1 - if (sscanf(data.c_str(), "%d", &ival) != 1 || ival < 0 || ival > 1) - throw options_warning_exception("Illegal boolean value for %s: \"%s\"; reverting to %s\n", name(), data, value()); + { + // booleans must be 0 or 1 + int ival; + if (!(str >> ival) || (0 > ival) || (1 < ival)) + throw options_warning_exception("Illegal boolean value for %s: \"%s\"; reverting to %s\n", name(), data, value()); + } break; case option_type::INTEGER: - // integers must be integral - if (sscanf(data.c_str(), "%d", &ival) != 1) - throw options_warning_exception("Illegal integer value for %s: \"%s\"; reverting to %s\n", name(), data, value()); - - // range checking - if (has_range()) { - int minimum_integer = atoi(minimum()); - int maximum_integer = atoi(maximum()); - if (ival < minimum_integer || ival > maximum_integer) - throw options_warning_exception("Out-of-range integer value for %s: \"%s\" (must be between %d and %d); reverting to %s\n", name(), data, minimum_integer, maximum_integer, value()); + // integers must be integral + int ival; + if (!(str >> ival)) + throw options_warning_exception("Illegal integer value for %s: \"%s\"; reverting to %s\n", name(), data, value()); + + // range checking + char const *const strmin(minimum()); + char const *const strmax(maximum()); + int imin(0), imax(0); + if (strmin) + { + str.str(strmin); + str >> imin; + } + if (strmax) + { + str.str(strmax); + str >> imax; + } + if ((strmin && (ival < imin)) || (strmax && (ival > imax))) + { + if (!strmax) + throw options_warning_exception("Out-of-range integer value for %s: \"%s\" (must be no less than %d); reverting to %s\n", name(), data, imin, value()); + else if (!strmin) + throw options_warning_exception("Out-of-range integer value for %s: \"%s\" (must be no greater than %d); reverting to %s\n", name(), data, imax, value()); + else + throw options_warning_exception("Out-of-range integer value for %s: \"%s\" (must be between %d and %d, inclusive); reverting to %s\n", name(), data, imin, imax, value()); + } } break; case option_type::FLOAT: - if (sscanf(data.c_str(), "%f", &fval) != 1) - throw options_warning_exception("Illegal float value for %s: \"%s\"; reverting to %s\n", name(), data, value()); - - // range checking - if (has_range()) { - float minimum_float = atof(minimum()); - float maximum_float = atof(maximum()); - if (fval < minimum_float || fval > maximum_float) - throw options_warning_exception("Out-of-range float value for %s: \"%s\" (must be between %f and %f); reverting to %s\n", name(), data, minimum_float, maximum_float, value()); + float fval; + if (!(str >> fval)) + throw options_warning_exception("Illegal float value for %s: \"%s\"; reverting to %s\n", name(), data, value()); + + // range checking + char const *const strmin(minimum()); + char const *const strmax(maximum()); + float fmin(0), fmax(0); + if (strmin) + { + str.str(strmin); + str >> fmin; + } + if (strmax) + { + str.str(strmax); + str >> fmax; + } + if ((strmin && (fval < fmin)) || (strmax && (fval > fmax))) + { + if (!strmax) + throw options_warning_exception("Out-of-range float value for %s: \"%s\" (must be no less than %f); reverting to %s\n", name(), data, fmin, value()); + else if (!strmin) + throw options_warning_exception("Out-of-range float value for %s: \"%s\" (must be no greater than %f); reverting to %s\n", name(), data, fmax, value()); + else + throw options_warning_exception("Out-of-range float value for %s: \"%s\" (must be between %f and %f, inclusive); reverting to %s\n", name(), data, fmin, fmax, value()); + } } break; @@ -238,7 +281,7 @@ void core_options::entry::validate(const std::string &data) // entry::minimum //------------------------------------------------- -const char *core_options::entry::minimum() const +const char *core_options::entry::minimum() const noexcept { return nullptr; } @@ -248,7 +291,7 @@ const char *core_options::entry::minimum() const // entry::maximum //------------------------------------------------- -const char *core_options::entry::maximum() const +const char *core_options::entry::maximum() const noexcept { return nullptr; } @@ -258,7 +301,7 @@ const char *core_options::entry::maximum() const // entry::has_range //------------------------------------------------- -bool core_options::entry::has_range() const +bool core_options::entry::has_range() const noexcept { return minimum() && maximum(); } @@ -268,11 +311,10 @@ bool core_options::entry::has_range() const // entry::default_value //------------------------------------------------- -const std::string &core_options::entry::default_value() const +const std::string &core_options::entry::default_value() const noexcept { - // I don't really want this generally available, but MewUI seems to need it. Please - // do not use - throw false; + // I don't really want this generally available, but MewUI seems to need it. Please do not use. + abort(); } @@ -307,25 +349,21 @@ core_options::simple_entry::~simple_entry() // simple_entry::value //------------------------------------------------- -const char *core_options::simple_entry::value() const +const char *core_options::simple_entry::value() const noexcept { - const char *result; switch (type()) { case core_options::option_type::BOOLEAN: case core_options::option_type::INTEGER: case core_options::option_type::FLOAT: case core_options::option_type::STRING: - result = m_data.c_str(); - break; + return m_data.c_str(); default: // this is an option type for which returning a value is // a meaningless operation (e.g. - core_options::option_type::COMMAND) - result = nullptr; - break; + return nullptr; } - return result; } @@ -333,7 +371,7 @@ const char *core_options::simple_entry::value() const // simple_entry::default_value //------------------------------------------------- -const std::string &core_options::simple_entry::default_value() const +const std::string &core_options::simple_entry::default_value() const noexcept { // only MewUI seems to need this; please don't use return m_defdata; @@ -364,7 +402,7 @@ void core_options::simple_entry::set_default_value(std::string &&newvalue) // minimum //------------------------------------------------- -const char *core_options::simple_entry::minimum() const +const char *core_options::simple_entry::minimum() const noexcept { return m_minimum.c_str(); } @@ -374,7 +412,7 @@ const char *core_options::simple_entry::minimum() const // maximum //------------------------------------------------- -const char *core_options::simple_entry::maximum() const +const char *core_options::simple_entry::maximum() const noexcept { return m_maximum.c_str(); } @@ -777,7 +815,7 @@ void core_options::copy_from(const core_options &that) if (dest_entry->names().size() > 0) { // identify the source entry - const entry::shared_ptr source_entry = that.get_entry(dest_entry->name()); + const entry::shared_const_ptr source_entry = that.get_entry(dest_entry->name()); if (source_entry) { const char *value = source_entry->value(); @@ -799,6 +837,7 @@ std::string core_options::output_ini(const core_options *diff) const { // INI files are complete, so always start with a blank buffer std::ostringstream buffer; + buffer.imbue(std::locale::classic()); int num_valid_headers = 0; int unadorned_index = 0; @@ -885,9 +924,10 @@ std::string core_options::output_help() const // value - return the raw option value //------------------------------------------------- -const char *core_options::value(const char *option) const +const char *core_options::value(const char *option) const noexcept { - return get_entry(option)->value(); + auto const entry = get_entry(option); + return entry ? entry->value() : nullptr; } @@ -895,9 +935,48 @@ const char *core_options::value(const char *option) const // description - return description of option //------------------------------------------------- -const char *core_options::description(const char *option) const +const char *core_options::description(const char *option) const noexcept +{ + auto const entry = get_entry(option); + return entry ? entry->description() : nullptr; +} + + +//------------------------------------------------- +// value - return the option value as an integer +//------------------------------------------------- + +int core_options::int_value(const char *option) const { - return get_entry(option)->description(); + char const *const data = value(option); + if (!data) + return 0; + std::istringstream str(data); + str.imbue(std::locale::classic()); + int ival; + if (str >> ival) + return ival; + else + return 0; +} + + +//------------------------------------------------- +// value - return the option value as a float +//------------------------------------------------- + +float core_options::float_value(const char *option) const +{ + char const *const data = value(option); + if (!data) + return 0.0f; + std::istringstream str(data); + str.imbue(std::locale::classic()); + float fval; + if (str >> fval) + return fval; + else + return 0.0f; } @@ -984,13 +1063,13 @@ void core_options::do_set_value(entry &curentry, std::string &&data, int priorit // get_entry //------------------------------------------------- -const core_options::entry::shared_ptr core_options::get_entry(const std::string &name) const +core_options::entry::shared_const_ptr core_options::get_entry(const std::string &name) const noexcept { auto curentry = m_entrymap.find(name); return (curentry != m_entrymap.end()) ? curentry->second.lock() : nullptr; } -core_options::entry::shared_ptr core_options::get_entry(const std::string &name) +core_options::entry::shared_ptr core_options::get_entry(const std::string &name) noexcept { auto curentry = m_entrymap.find(name); return (curentry != m_entrymap.end()) ? curentry->second.lock() : nullptr; @@ -1011,17 +1090,17 @@ void core_options::set_value_changed_handler(const std::string &name, std::funct // header_exists //------------------------------------------------- -bool core_options::header_exists(const char *description) const +bool core_options::header_exists(const char *description) const noexcept { auto iter = std::find_if( - m_entries.begin(), - m_entries.end(), - [description](const auto &entry) - { - return entry->type() == option_type::HEADER - && entry->description() - && !strcmp(entry->description(), description); - }); + m_entries.begin(), + m_entries.end(), + [description](const auto &entry) + { + return entry->type() == option_type::HEADER + && entry->description() + && !strcmp(entry->description(), description); + }); return iter != m_entries.end(); } diff --git a/src/lib/util/options.h b/src/lib/util/options.h index 3fcd9d5e144..457ec08dd1d 100644 --- a/src/lib/util/options.h +++ b/src/lib/util/options.h @@ -101,6 +101,7 @@ public: { public: typedef std::shared_ptr shared_ptr; + typedef std::shared_ptr shared_const_ptr; typedef std::weak_ptr weak_ptr; // constructor/destructor @@ -113,17 +114,17 @@ public: virtual ~entry(); // accessors - const std::vector &names() const { return m_names; } - const std::string &name() const { return m_names[0]; } - virtual const char *value() const; - int priority() const { return m_priority; } - void set_priority(int priority) { m_priority = priority; } - option_type type() const { return m_type; } - const char *description() const { return m_description; } - virtual const std::string &default_value() const; - virtual const char *minimum() const; - virtual const char *maximum() const; - bool has_range() const; + const std::vector &names() const noexcept { return m_names; } + const std::string &name() const noexcept { return m_names[0]; } + virtual const char *value() const noexcept; + int priority() const noexcept { return m_priority; } + void set_priority(int priority) noexcept { m_priority = priority; } + option_type type() const noexcept { return m_type; } + const char *description() const noexcept { return m_description; } + virtual const std::string &default_value() const noexcept; + virtual const char *minimum() const noexcept; + virtual const char *maximum() const noexcept; + bool has_range() const noexcept; // mutators void set_value(std::string &&newvalue, int priority, bool always_override = false); @@ -154,13 +155,13 @@ public: virtual ~core_options(); // getters - const std::string &command() const { return m_command; } - const std::vector &command_arguments() const { assert(!m_command.empty()); return m_command_arguments; } - const entry::shared_ptr get_entry(const std::string &name) const; - entry::shared_ptr get_entry(const std::string &name); - const std::vector &entries() const { return m_entries; } - bool exists(const std::string &name) const { return get_entry(name) != nullptr; } - bool header_exists(const char *description) const; + const std::string &command() const noexcept { return m_command; } + const std::vector &command_arguments() const noexcept { assert(!m_command.empty()); return m_command_arguments; } + entry::shared_const_ptr get_entry(const std::string &name) const noexcept; + entry::shared_ptr get_entry(const std::string &name) noexcept; + const std::vector &entries() const noexcept { return m_entries; } + bool exists(const std::string &name) const noexcept { return get_entry(name) != nullptr; } + bool header_exists(const char *description) const noexcept; // configuration void add_entry(entry::shared_ptr &&entry, const char *after_header = nullptr); @@ -184,11 +185,11 @@ public: std::string output_help() const; // reading - const char *value(const char *option) const; - const char *description(const char *option) const; + const char *value(const char *option) const noexcept; + const char *description(const char *option) const noexcept; bool bool_value(const char *option) const { return int_value(option) != 0; } - int int_value(const char *option) const { return atoi(value(option)); } - float float_value(const char *option) const { return atof(value(option)); } + int int_value(const char *option) const; + float float_value(const char *option) const; // setting void set_value(const std::string &name, const std::string &value, int priority); @@ -197,7 +198,7 @@ public: void set_value(const std::string &name, float value, int priority); // misc - static const char *unadorned(int x = 0) { return s_option_unadorned[std::min(x, MAX_UNADORNED_OPTIONS - 1)]; } + static const char *unadorned(int x = 0) noexcept { return s_option_unadorned[std::min(x, MAX_UNADORNED_OPTIONS - 1)]; } protected: virtual void command_argument_processed() { } @@ -215,10 +216,10 @@ private: ~simple_entry(); // getters - virtual const char *value() const override; - virtual const char *minimum() const override; - virtual const char *maximum() const override; - virtual const std::string &default_value() const override; + virtual const char *value() const noexcept override; + virtual const char *minimum() const noexcept override; + virtual const char *maximum() const noexcept override; + virtual const std::string &default_value() const noexcept override; virtual void revert(int priority_hi, int priority_lo) override; virtual void set_default_value(std::string &&newvalue) override; -- cgit v1.2.3