util/options.cpp: fix locale issues and a const correctness issue

This commit is contained in:
Vas Crabb 2019-11-23 11:30:07 +11:00
parent aea6bbe2b4
commit e09f2e49a6
4 changed files with 175 additions and 94 deletions

View File

@ -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 // This is returning an empty string instead of nullptr to signify that
// specifying the value is a meaningful operation. The option types 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; const char *result = nullptr;
if (m_host.specified()) if (m_host.specified())
@ -298,6 +298,7 @@ namespace
// happen in practice // happen in practice
// //
// In reality, I want to really return std::optional<std::string> here // In reality, I want to really return std::optional<std::string> here
// FIXME: the std::string assignment can throw exceptions, and returning std::optional<std::string> also isn't safe in noexcept
m_temp = m_host.specified_value(); m_temp = m_host.specified_value();
result = m_temp.c_str(); 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(); return m_host.value().c_str();
} }

View File

@ -96,7 +96,7 @@ ui_options::ui_options() : core_options()
rgb_t ui_options::rgb_value(const char *option) const rgb_t ui_options::rgb_value(const char *option) const
{ {
// find the entry // 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 // look up the value, and sanity check the result
const char *value = entry->value(); const char *value = entry->value();

View File

@ -8,13 +8,17 @@
***************************************************************************/ ***************************************************************************/
#include "options.h"
#include "corestr.h"
#include <locale>
#include <string>
#include <assert.h>
#include <ctype.h>
#include <stdarg.h> #include <stdarg.h>
#include <stdlib.h> #include <stdlib.h>
#include <ctype.h>
#include <assert.h>
#include "options.h"
#include "corestr.h"
#include <string>
const int core_options::MAX_UNADORNED_OPTIONS; const int core_options::MAX_UNADORNED_OPTIONS;
@ -134,7 +138,7 @@ core_options::entry::~entry()
// entry::value // 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" // 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) // 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) void core_options::entry::validate(const std::string &data)
{ {
float fval; std::istringstream str(data);
int ival; str.imbue(std::locale::classic());
switch (type()) switch (type())
{ {
case option_type::BOOLEAN: case option_type::BOOLEAN:
{
// booleans must be 0 or 1 // booleans must be 0 or 1
if (sscanf(data.c_str(), "%d", &ival) != 1 || ival < 0 || ival > 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()); throw options_warning_exception("Illegal boolean value for %s: \"%s\"; reverting to %s\n", name(), data, value());
}
break; break;
case option_type::INTEGER: case option_type::INTEGER:
{
// integers must be integral // integers must be integral
if (sscanf(data.c_str(), "%d", &ival) != 1) int ival;
if (!(str >> ival))
throw options_warning_exception("Illegal integer value for %s: \"%s\"; reverting to %s\n", name(), data, value()); throw options_warning_exception("Illegal integer value for %s: \"%s\"; reverting to %s\n", name(), data, value());
// range checking // range checking
if (has_range()) char const *const strmin(minimum());
char const *const strmax(maximum());
int imin(0), imax(0);
if (strmin)
{ {
int minimum_integer = atoi(minimum()); str.str(strmin);
int maximum_integer = atoi(maximum()); str >> imin;
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()); 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; break;
case option_type::FLOAT: case option_type::FLOAT:
if (sscanf(data.c_str(), "%f", &fval) != 1) {
float fval;
if (!(str >> fval))
throw options_warning_exception("Illegal float value for %s: \"%s\"; reverting to %s\n", name(), data, value()); throw options_warning_exception("Illegal float value for %s: \"%s\"; reverting to %s\n", name(), data, value());
// range checking // range checking
if (has_range()) char const *const strmin(minimum());
char const *const strmax(maximum());
float fmin(0), fmax(0);
if (strmin)
{ {
float minimum_float = atof(minimum()); str.str(strmin);
float maximum_float = atof(maximum()); str >> fmin;
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()); 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; break;
@ -238,7 +281,7 @@ void core_options::entry::validate(const std::string &data)
// entry::minimum // entry::minimum
//------------------------------------------------- //-------------------------------------------------
const char *core_options::entry::minimum() const const char *core_options::entry::minimum() const noexcept
{ {
return nullptr; return nullptr;
} }
@ -248,7 +291,7 @@ const char *core_options::entry::minimum() const
// entry::maximum // entry::maximum
//------------------------------------------------- //-------------------------------------------------
const char *core_options::entry::maximum() const const char *core_options::entry::maximum() const noexcept
{ {
return nullptr; return nullptr;
} }
@ -258,7 +301,7 @@ const char *core_options::entry::maximum() const
// entry::has_range // entry::has_range
//------------------------------------------------- //-------------------------------------------------
bool core_options::entry::has_range() const bool core_options::entry::has_range() const noexcept
{ {
return minimum() && maximum(); return minimum() && maximum();
} }
@ -268,11 +311,10 @@ bool core_options::entry::has_range() const
// entry::default_value // 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 // I don't really want this generally available, but MewUI seems to need it. Please do not use.
// do not use abort();
throw false;
} }
@ -307,25 +349,21 @@ core_options::simple_entry::~simple_entry()
// simple_entry::value // 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()) switch (type())
{ {
case core_options::option_type::BOOLEAN: case core_options::option_type::BOOLEAN:
case core_options::option_type::INTEGER: case core_options::option_type::INTEGER:
case core_options::option_type::FLOAT: case core_options::option_type::FLOAT:
case core_options::option_type::STRING: case core_options::option_type::STRING:
result = m_data.c_str(); return m_data.c_str();
break;
default: default:
// this is an option type for which returning a value is // this is an option type for which returning a value is
// a meaningless operation (e.g. - core_options::option_type::COMMAND) // a meaningless operation (e.g. - core_options::option_type::COMMAND)
result = nullptr; return nullptr;
break;
} }
return result;
} }
@ -333,7 +371,7 @@ const char *core_options::simple_entry::value() const
// simple_entry::default_value // 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 // only MewUI seems to need this; please don't use
return m_defdata; return m_defdata;
@ -364,7 +402,7 @@ void core_options::simple_entry::set_default_value(std::string &&newvalue)
// minimum // minimum
//------------------------------------------------- //-------------------------------------------------
const char *core_options::simple_entry::minimum() const const char *core_options::simple_entry::minimum() const noexcept
{ {
return m_minimum.c_str(); return m_minimum.c_str();
} }
@ -374,7 +412,7 @@ const char *core_options::simple_entry::minimum() const
// maximum // maximum
//------------------------------------------------- //-------------------------------------------------
const char *core_options::simple_entry::maximum() const const char *core_options::simple_entry::maximum() const noexcept
{ {
return m_maximum.c_str(); return m_maximum.c_str();
} }
@ -777,7 +815,7 @@ void core_options::copy_from(const core_options &that)
if (dest_entry->names().size() > 0) if (dest_entry->names().size() > 0)
{ {
// identify the source entry // 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) if (source_entry)
{ {
const char *value = source_entry->value(); 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 // INI files are complete, so always start with a blank buffer
std::ostringstream buffer; std::ostringstream buffer;
buffer.imbue(std::locale::classic());
int num_valid_headers = 0; int num_valid_headers = 0;
int unadorned_index = 0; int unadorned_index = 0;
@ -885,9 +924,10 @@ std::string core_options::output_help() const
// value - return the raw option value // 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 // description - return description of option
//------------------------------------------------- //-------------------------------------------------
const char *core_options::description(const char *option) const const char *core_options::description(const char *option) const noexcept
{ {
return get_entry(option)->description(); 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
{
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 // 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); auto curentry = m_entrymap.find(name);
return (curentry != m_entrymap.end()) ? curentry->second.lock() : nullptr; 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); auto curentry = m_entrymap.find(name);
return (curentry != m_entrymap.end()) ? curentry->second.lock() : nullptr; return (curentry != m_entrymap.end()) ? curentry->second.lock() : nullptr;
@ -1011,7 +1090,7 @@ void core_options::set_value_changed_handler(const std::string &name, std::funct
// header_exists // 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( auto iter = std::find_if(
m_entries.begin(), m_entries.begin(),

View File

@ -101,6 +101,7 @@ public:
{ {
public: public:
typedef std::shared_ptr<entry> shared_ptr; typedef std::shared_ptr<entry> shared_ptr;
typedef std::shared_ptr<const entry> shared_const_ptr;
typedef std::weak_ptr<entry> weak_ptr; typedef std::weak_ptr<entry> weak_ptr;
// constructor/destructor // constructor/destructor
@ -113,17 +114,17 @@ public:
virtual ~entry(); virtual ~entry();
// accessors // accessors
const std::vector<std::string> &names() const { return m_names; } const std::vector<std::string> &names() const noexcept { return m_names; }
const std::string &name() const { return m_names[0]; } const std::string &name() const noexcept { return m_names[0]; }
virtual const char *value() const; virtual const char *value() const noexcept;
int priority() const { return m_priority; } int priority() const noexcept { return m_priority; }
void set_priority(int priority) { m_priority = priority; } void set_priority(int priority) noexcept { m_priority = priority; }
option_type type() const { return m_type; } option_type type() const noexcept { return m_type; }
const char *description() const { return m_description; } const char *description() const noexcept { return m_description; }
virtual const std::string &default_value() const; virtual const std::string &default_value() const noexcept;
virtual const char *minimum() const; virtual const char *minimum() const noexcept;
virtual const char *maximum() const; virtual const char *maximum() const noexcept;
bool has_range() const; bool has_range() const noexcept;
// mutators // mutators
void set_value(std::string &&newvalue, int priority, bool always_override = false); void set_value(std::string &&newvalue, int priority, bool always_override = false);
@ -154,13 +155,13 @@ public:
virtual ~core_options(); virtual ~core_options();
// getters // getters
const std::string &command() const { return m_command; } const std::string &command() const noexcept { return m_command; }
const std::vector<std::string> &command_arguments() const { assert(!m_command.empty()); return m_command_arguments; } const std::vector<std::string> &command_arguments() const noexcept { assert(!m_command.empty()); return m_command_arguments; }
const entry::shared_ptr get_entry(const std::string &name) const; entry::shared_const_ptr get_entry(const std::string &name) const noexcept;
entry::shared_ptr get_entry(const std::string &name); entry::shared_ptr get_entry(const std::string &name) noexcept;
const std::vector<entry::shared_ptr> &entries() const { return m_entries; } const std::vector<entry::shared_ptr> &entries() const noexcept { return m_entries; }
bool exists(const std::string &name) const { return get_entry(name) != nullptr; } bool exists(const std::string &name) const noexcept { return get_entry(name) != nullptr; }
bool header_exists(const char *description) const; bool header_exists(const char *description) const noexcept;
// configuration // configuration
void add_entry(entry::shared_ptr &&entry, const char *after_header = nullptr); void add_entry(entry::shared_ptr &&entry, const char *after_header = nullptr);
@ -184,11 +185,11 @@ public:
std::string output_help() const; std::string output_help() const;
// reading // reading
const char *value(const char *option) const; const char *value(const char *option) const noexcept;
const char *description(const char *option) const; const char *description(const char *option) const noexcept;
bool bool_value(const char *option) const { return int_value(option) != 0; } bool bool_value(const char *option) const { return int_value(option) != 0; }
int int_value(const char *option) const { return atoi(value(option)); } int int_value(const char *option) const;
float float_value(const char *option) const { return atof(value(option)); } float float_value(const char *option) const;
// setting // setting
void set_value(const std::string &name, const std::string &value, int priority); 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); void set_value(const std::string &name, float value, int priority);
// misc // 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: protected:
virtual void command_argument_processed() { } virtual void command_argument_processed() { }
@ -215,10 +216,10 @@ private:
~simple_entry(); ~simple_entry();
// getters // getters
virtual const char *value() const override; virtual const char *value() const noexcept override;
virtual const char *minimum() const override; virtual const char *minimum() const noexcept override;
virtual const char *maximum() const override; virtual const char *maximum() const noexcept override;
virtual const std::string &default_value() const override; virtual const std::string &default_value() const noexcept override;
virtual void revert(int priority_hi, int priority_lo) override; virtual void revert(int priority_hi, int priority_lo) override;
virtual void set_default_value(std::string &&newvalue) override; virtual void set_default_value(std::string &&newvalue) override;