Bug fix to -romident and aux verb cleanup (#2288)

* Bug fix to -romident and aux verb cleanup

Made the following changes:
 1.  Fixed a bug where resolved slot/image options would choke -romident (reproducible in MAME 0.185 with 'mame64 -romident coco.zip')
 2.  'mame64 -romident' no longer crashes (though it doesn't do anything useful)
 3.  Changed the aux verb functions to take 'const std::string &'

* Further cleanups to auxillary verb code, as per Vas

Specifically:
 1.  The commands themselves now take 'const std::vector<std::string> &' for their argument lists
 2.  util::core_options now collects command arguments into a separate vector rather than treating them as unadorned arguments

* Vas Crabb feedback

* Now only using trim_spaces_and_quotes() when parsing INIs

Vas pointed out that it is inappropriate to trim spaces and quotes when parsing command line options
This commit is contained in:
npwoods 2017-05-11 19:18:20 -04:00 committed by Vas Crabb
parent 16cbbd9103
commit 78bf804192
4 changed files with 150 additions and 80 deletions

View File

@ -2,7 +2,7 @@
// copyright-holders:Aaron Giles
/***************************************************************************
clifront.c
clifront.cpp
Command-line interface frontend for MAME.
@ -317,12 +317,14 @@ int cli_frontend::execute(std::vector<std::string> &args)
// games
//-------------------------------------------------
void cli_frontend::listxml(const char *gamename)
void cli_frontend::listxml(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
throw emu_fatalerror(EMU_ERR_NO_SUCH_GAME, "No matching games found for '%s'", gamename);
throw emu_fatalerror(EMU_ERR_NO_SUCH_GAME, "No matching games found for '%s'", gamename ? gamename : "");
// create the XML and print it to stdout
info_xml_creator creator(drivlist, gamename && *gamename);
@ -335,8 +337,10 @@ void cli_frontend::listxml(const char *gamename)
// one or more games
//-------------------------------------------------
void cli_frontend::listfull(const char *gamename)
void cli_frontend::listfull(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -357,8 +361,10 @@ void cli_frontend::listfull(const char *gamename)
// filename of one or more games
//-------------------------------------------------
void cli_frontend::listsource(const char *gamename)
void cli_frontend::listsource(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -375,8 +381,10 @@ void cli_frontend::listsource(const char *gamename)
// clones matching the given pattern
//-------------------------------------------------
void cli_frontend::listclones(const char *gamename)
void cli_frontend::listclones(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// start with a filtered list of drivers
driver_enumerator drivlist(m_options, gamename);
int original_count = drivlist.count();
@ -422,8 +430,10 @@ void cli_frontend::listclones(const char *gamename)
// source file
//-------------------------------------------------
void cli_frontend::listbrothers(const char *gamename)
void cli_frontend::listbrothers(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// start with a filtered list of drivers; return an error if none found
driver_enumerator initial_drivlist(m_options, gamename);
if (initial_drivlist.count() == 0)
@ -465,8 +475,10 @@ void cli_frontend::listbrothers(const char *gamename)
// referenced by the emulator
//-------------------------------------------------
void cli_frontend::listcrc(const char *gamename)
void cli_frontend::listcrc(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -493,8 +505,10 @@ void cli_frontend::listcrc(const char *gamename)
// by a given game or set of games
//-------------------------------------------------
void cli_frontend::listroms(const char *gamename)
void cli_frontend::listroms(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -554,8 +568,10 @@ void cli_frontend::listroms(const char *gamename)
// referenced by a given game or set of games
//-------------------------------------------------
void cli_frontend::listsamples(const char *gamename)
void cli_frontend::listsamples(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -592,8 +608,10 @@ void cli_frontend::listsamples(const char *gamename)
// referenced by a given game or set of games
//-------------------------------------------------
void cli_frontend::listdevices(const char *gamename)
void cli_frontend::listdevices(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -667,8 +685,10 @@ void cli_frontend::listdevices(const char *gamename)
// referenced by a given game or set of games
//-------------------------------------------------
void cli_frontend::listslots(const char *gamename)
void cli_frontend::listslots(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -725,8 +745,10 @@ void cli_frontend::listslots(const char *gamename)
// referenced by a given game or set of games
//-------------------------------------------------
void cli_frontend::listmedia(const char *gamename)
void cli_frontend::listmedia(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
if (drivlist.count() == 0)
@ -777,8 +799,10 @@ void cli_frontend::listmedia(const char *gamename)
// verifyroms - verify the ROM sets of one or
// more games
//-------------------------------------------------
void cli_frontend::verifyroms(const char *gamename)
void cli_frontend::verifyroms(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
// determine which drivers to output;
driver_enumerator drivlist(m_options, gamename);
@ -856,10 +880,9 @@ void cli_frontend::verifyroms(const char *gamename)
// one or more games
//-------------------------------------------------
void cli_frontend::verifysamples(const char *gamename)
void cli_frontend::verifysamples(const std::vector<std::string> &args)
{
if (!gamename)
gamename = "*";
const char *gamename = args.empty() ? "*" : args[0].c_str();
// determine which drivers to output; return an error if none found
driver_enumerator drivlist(m_options, gamename);
@ -1093,8 +1116,10 @@ void cli_frontend::output_single_softlist(FILE *out, software_list_device &swlis
identifying duplicate lists.
-------------------------------------------------*/
void cli_frontend::listsoftware(const char *gamename)
void cli_frontend::listsoftware(const std::vector<std::string> &args)
{
const char *gamename = args.empty() ? nullptr : args[0].c_str();
FILE *out = stdout;
std::unordered_set<std::string> list_map;
bool isfirst = true;
@ -1126,10 +1151,9 @@ void cli_frontend::listsoftware(const char *gamename)
verifysoftware - verify roms from the software
list of the specified driver(s)
-------------------------------------------------*/
void cli_frontend::verifysoftware(const char *gamename)
void cli_frontend::verifysoftware(const std::vector<std::string> &args)
{
if (!gamename)
gamename = "*";
const char *gamename = args.size() > 0 ? args[0].c_str() : "*";
std::unordered_set<std::string> list_map;
@ -1203,10 +1227,9 @@ void cli_frontend::verifysoftware(const char *gamename)
getsoftlist - retrieve software list by name
-------------------------------------------------*/
void cli_frontend::getsoftlist(const char *gamename)
void cli_frontend::getsoftlist(const std::vector<std::string> &args)
{
if (!gamename)
gamename = "*";
const char *gamename = args.size() > 0 ? args[0].c_str() : "*";
FILE *out = stdout;
std::unordered_set<std::string> list_map;
@ -1234,10 +1257,9 @@ void cli_frontend::getsoftlist(const char *gamename)
/*-------------------------------------------------
verifysoftlist - verify software list by name
-------------------------------------------------*/
void cli_frontend::verifysoftlist(const char *gamename)
void cli_frontend::verifysoftlist(const std::vector<std::string> &args)
{
if (!gamename)
gamename = "*";
const char *gamename = args.size() > 0 ? args[0].c_str() : "*";
std::unordered_set<std::string> list_map;
unsigned correct = 0;
@ -1301,9 +1323,17 @@ void cli_frontend::verifysoftlist(const char *gamename)
// matches in our internal database
//-------------------------------------------------
void cli_frontend::romident(const char *filename)
void cli_frontend::romident(const std::vector<std::string> &args)
{
media_identifier ident(m_options);
const char *filename = args[0].c_str();
// create our own copy of options for the purposes of ROM identification
// so we are not "polluted" with driver-specific slot/image options
emu_options options;
std::string error_string;
options.set_value(OPTION_MEDIAPATH, m_options.media_path(), OPTION_PRIORITY_DEFAULT, error_string);
media_identifier ident(options);
// identify the file, then output results
osd_printf_info("Identifying %s....\n", filename);
@ -1409,27 +1439,30 @@ void cli_frontend::execute_commands(const char *exename)
static const struct
{
const char *option;
void (cli_frontend::*function)(const char *gamename);
int min_args;
int max_args;
void (cli_frontend::*function)(const std::vector<std::string> &args);
const char *usage;
} info_commands[] =
{
{ CLICOMMAND_LISTXML, &cli_frontend::listxml },
{ CLICOMMAND_LISTFULL, &cli_frontend::listfull },
{ CLICOMMAND_LISTSOURCE, &cli_frontend::listsource },
{ CLICOMMAND_LISTCLONES, &cli_frontend::listclones },
{ CLICOMMAND_LISTBROTHERS, &cli_frontend::listbrothers },
{ CLICOMMAND_LISTCRC, &cli_frontend::listcrc },
{ CLICOMMAND_LISTDEVICES, &cli_frontend::listdevices },
{ CLICOMMAND_LISTSLOTS, &cli_frontend::listslots },
{ CLICOMMAND_LISTROMS, &cli_frontend::listroms },
{ CLICOMMAND_LISTSAMPLES, &cli_frontend::listsamples },
{ CLICOMMAND_VERIFYROMS, &cli_frontend::verifyroms },
{ CLICOMMAND_VERIFYSAMPLES, &cli_frontend::verifysamples },
{ CLICOMMAND_LISTMEDIA, &cli_frontend::listmedia },
{ CLICOMMAND_LISTSOFTWARE, &cli_frontend::listsoftware },
{ CLICOMMAND_VERIFYSOFTWARE,&cli_frontend::verifysoftware },
{ CLICOMMAND_ROMIDENT, &cli_frontend::romident },
{ CLICOMMAND_GETSOFTLIST, &cli_frontend::getsoftlist },
{ CLICOMMAND_VERIFYSOFTLIST,&cli_frontend::verifysoftlist },
{ CLICOMMAND_LISTXML, 0, 1, &cli_frontend::listxml, "[system name]" },
{ CLICOMMAND_LISTFULL, 0, 1, &cli_frontend::listfull, "[system name]" },
{ CLICOMMAND_LISTSOURCE, 0, 1, &cli_frontend::listsource, "[system name]" },
{ CLICOMMAND_LISTCLONES, 0, 1, &cli_frontend::listclones, "[system name]" },
{ CLICOMMAND_LISTBROTHERS, 0, 1, &cli_frontend::listbrothers, "[system name]" },
{ CLICOMMAND_LISTCRC, 0, 1, &cli_frontend::listcrc, "[system name]" },
{ CLICOMMAND_LISTDEVICES, 0, 1, &cli_frontend::listdevices, "[system name]" },
{ CLICOMMAND_LISTSLOTS, 0, 1, &cli_frontend::listslots, "[system name]" },
{ CLICOMMAND_LISTROMS, 0, 1, &cli_frontend::listroms, "[system name]" },
{ CLICOMMAND_LISTSAMPLES, 0, 1, &cli_frontend::listsamples, "[system name]" },
{ CLICOMMAND_VERIFYROMS, 0, 1, &cli_frontend::verifyroms, "[system name]" },
{ CLICOMMAND_VERIFYSAMPLES, 0, 1, &cli_frontend::verifysamples, "[system name|*]" },
{ CLICOMMAND_LISTMEDIA, 0, 1, &cli_frontend::listmedia, "[system name]" },
{ CLICOMMAND_LISTSOFTWARE, 0, 1, &cli_frontend::listsoftware, "[system name]" },
{ CLICOMMAND_VERIFYSOFTWARE, 0, 1, &cli_frontend::verifysoftware, "[system name|*]" },
{ CLICOMMAND_ROMIDENT, 1, 1, &cli_frontend::romident, "(system name)" },
{ CLICOMMAND_GETSOFTLIST, 0, 1, &cli_frontend::getsoftlist, "[system name|*]" },
{ CLICOMMAND_VERIFYSOFTLIST, 0, 1, &cli_frontend::verifysoftlist, "[system name|*]" },
};
// find the command
@ -1437,9 +1470,22 @@ void cli_frontend::execute_commands(const char *exename)
{
if (m_options.command() == info_command.option)
{
// parse any relevant INI files before proceeding
const char *sysname = m_options.system_name();
(this->*info_command.function)((sysname[0] == 0) ? nullptr : sysname);
// validate argument count
const char *error_message = nullptr;
if (m_options.command_arguments().size() < info_command.min_args)
error_message = "Auxillary verb -%s requires at least %d argument(s)\n";
if (m_options.command_arguments().size() > info_command.max_args)
error_message = "Auxillary verb -%s takes at most %d argument(s)\n";
if (error_message)
{
osd_printf_info(error_message, info_command.option, info_command.max_args);
osd_printf_info("\n");
osd_printf_info("Usage: %s -%s %s\n", exename, info_command.option, info_command.usage);
return;
}
// invoke the auxillary command!
(this->*info_command.function)(m_options.command_arguments());
return;
}
}

View File

@ -34,26 +34,28 @@ public:
int execute(std::vector<std::string> &args);
// direct access to the command operations
void listxml(const char *gamename = "*");
void listfull(const char *gamename = "*");
void listsource(const char *gamename = "*");
void listclones(const char *gamename = "*");
void listbrothers(const char *gamename = "*");
void listcrc(const char *gamename = "*");
void listroms(const char *gamename = "*");
void listsamples(const char *gamename = "*");
void listdevices(const char *gamename = "*");
void listslots(const char *gamename = "*");
void listmedia(const char *gamename = "*");
void listsoftware(const char *gamename = "*");
void verifysoftware(const char *gamename = "*");
void verifyroms(const char *gamename = "*");
void verifysamples(const char *gamename = "*");
void romident(const char *filename);
void getsoftlist(const char *gamename = "*");
void verifysoftlist(const char *gamename = "*");
private:
// commands
void listxml(const std::vector<std::string> &args);
void listfull(const std::vector<std::string> &args);
void listsource(const std::vector<std::string> &args);
void listclones(const std::vector<std::string> &args);
void listbrothers(const std::vector<std::string> &args);
void listcrc(const std::vector<std::string> &args);
void listroms(const std::vector<std::string> &args);
void listsamples(const std::vector<std::string> &args);
void listdevices(const std::vector<std::string> &args);
void listslots(const std::vector<std::string> &args);
void listmedia(const std::vector<std::string> &args);
void listsoftware(const std::vector<std::string> &args);
void verifysoftware(const std::vector<std::string> &args);
void verifyroms(const std::vector<std::string> &args);
void verifysamples(const std::vector<std::string> &args);
void romident(const std::vector<std::string> &args);
void getsoftlist(const std::vector<std::string> &args);
void verifysoftlist(const std::vector<std::string> &args);
// internal helpers
void execute_commands(const char *exename);
void display_help(const char *exename);

View File

@ -44,6 +44,26 @@ const char *const core_options::s_option_unadorned[MAX_UNADORNED_OPTIONS] =
};
//**************************************************************************
// UTILITY
//**************************************************************************
namespace
{
void trim_spaces_and_quotes(std::string &data)
{
// trim any whitespace
strtrimspace(data);
// trim quotes
if (data.find_first_of('"') == 0 && data.find_last_of('"') == data.length() - 1)
{
data.erase(0, 1);
data.erase(data.length() - 1, 1);
}
}
};
//**************************************************************************
// CORE OPTIONS ENTRY
@ -353,6 +373,14 @@ bool core_options::parse_command_line(std::vector<std::string> &args, int priori
bool is_unadorned = (curarg[0] != '-');
const char *optionname = is_unadorned ? core_options::unadorned(unadorned_index++) : &curarg[1];
// special case - collect unadorned arguments after commands into a special place
if (is_unadorned && !m_command.empty())
{
m_command_arguments.push_back(std::move(args[arg]));
args[arg].clear();
continue;
}
// find our entry; if not found, continue
auto curentry = m_entrymap.find(optionname);
if (curentry == m_entrymap.end())
@ -478,7 +506,9 @@ bool core_options::parse_ini_file(util::core_file &inifile, int priority, bool i
}
// set the new data
validate_and_set_data(*curentry->second, optiondata, priority, error_string);
std::string data = optiondata;
trim_spaces_and_quotes(data);
validate_and_set_data(*curentry->second, std::move(data), priority, error_string);
}
return true;
}
@ -832,16 +862,6 @@ void core_options::copyfrom(const core_options &src)
bool core_options::validate_and_set_data(core_options::entry &curentry, std::string &&data, int priority, std::string &error_string)
{
// trim any whitespace
strtrimspace(data);
// trim quotes
if (data.find_first_of('"') == 0 && data.find_last_of('"') == data.length() - 1)
{
data.erase(0, 1);
data.erase(data.length() - 1, 1);
}
// let derived classes override how we set this data
if (override_set_value(curentry.name(), data))
return true;

View File

@ -129,6 +129,7 @@ public:
// getters
entry *first() const { return m_entrylist.first(); }
const std::string &command() const { return m_command; }
const std::vector<std::string> &command_arguments() const { assert(!m_command.empty()); return m_command_arguments; }
entry *get_entry(const char *name) const;
// range iterators
@ -201,6 +202,7 @@ private:
simple_list<entry> m_entrylist; // head of list of entries
std::unordered_map<std::string,entry *> m_entrymap; // map for fast lookup
std::string m_command; // command found
std::vector<std::string> m_command_arguments; // command arguments
static const char *const s_option_unadorned[]; // array of unadorned option "names"
};