From dda1cf6a74c04b0a9befdd94ff55fe3709b6afa8 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 25 Sep 2009 19:51:35 +0000 Subject: [PATCH] Fixed buggy argument handling in GSoC tools code svn-id: r44368 --- compress.cpp | 105 +++++++++++++++++++++------------------ compress_agos.cpp | 4 +- compress_gob.cpp | 4 +- compress_sword1.cpp | 8 +-- example_tool.cpp | 11 ++-- extract_kyra.cpp | 15 +++--- extract_parallaction.cpp | 4 +- tool.cpp | 50 +++++++------------ tool.h | 15 ++---- tools_cli.cpp | 6 +-- 10 files changed, 103 insertions(+), 119 deletions(-) diff --git a/compress.cpp b/compress.cpp index fb12341a..68963820 100644 --- a/compress.cpp +++ b/compress.cpp @@ -670,33 +670,34 @@ void CompressionTool::extractAndEncodeVOC(const char *outName, File &input, Audi } bool CompressionTool::processMp3Parms() { - while (_arguments_parsed < _arguments.size()) { - std::string arg = _arguments[_arguments_parsed]; + while (!_arguments.empty()) { + std::string arg = _arguments.front(); + _arguments.pop_front(); if (arg == "--vbr") { encparms.abr = 0; } else if (arg == "--abr") { encparms.abr = 1; } else if (arg == "-b") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -b"); - encparms.minBitr = atoi(_arguments[_arguments_parsed].c_str()); + encparms.minBitr = atoi(_arguments.front().c_str()); if (encparms.minBitr > 160) throw ToolException("Minimum bitrate out of bounds (-b), must be between 8 and 160."); - if (encparms.minBitr == 0 && _arguments[_arguments_parsed] != "0") + if (encparms.minBitr == 0 && _arguments.front() != "0") throw ToolException("Minimum bitrate (-b) must be a number."); if (encparms.minBitr < 8) throw ToolException("Minimum bitrate out of bounds (-b), must be between 8 and 160."); + _arguments.pop_front(); + } else if (arg == "-B") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -B"); - encparms.maxBitr = atoi(_arguments[_arguments_parsed].c_str()); + encparms.maxBitr = atoi(_arguments.front().c_str()); if ((encparms.maxBitr % 8) != 0) { encparms.maxBitr -= encparms.maxBitr % 8; @@ -705,17 +706,18 @@ bool CompressionTool::processMp3Parms() { if (encparms.maxBitr > 160) throw ToolException("Maximum bitrate out of bounds (-B), must be between 8 and 160."); - if (encparms.maxBitr == 0 && _arguments[_arguments_parsed] != "0") + if (encparms.maxBitr == 0 && _arguments.front() != "0") throw ToolException("Maximum bitrate (-B) must be a number."); if (encparms.maxBitr < 8) throw ToolException("Maximum bitrate out of bounds (-B), must be between 8 and 160."); + _arguments.pop_front(); + } else if (arg == "-V") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -V"); - encparms.vbrqual = atoi(_arguments[_arguments_parsed].c_str()); + encparms.vbrqual = atoi(_arguments.front().c_str()); if (encparms.vbrqual < 0) throw ToolException("Quality (-q) out of bounds, must be between 0 and 9."); @@ -723,11 +725,12 @@ bool CompressionTool::processMp3Parms() { if (encparms.vbrqual > 9) throw ToolException("Quality (-q) out of bounds, must be between 0 and 9."); + _arguments.pop_front(); + } else if (arg == "-q") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -q"); - encparms.algqual = atoi(_arguments[_arguments_parsed].c_str()); + encparms.algqual = atoi(_arguments.front().c_str()); if (encparms.algqual < 0) throw ToolException("Quality (-q) out of bounds, must be between 0 and 9."); @@ -735,27 +738,27 @@ bool CompressionTool::processMp3Parms() { if (encparms.algqual > 9) throw ToolException("Quality (-q) out of bounds, must be between 0 and 9."); + _arguments.pop_front(); + } else if (arg == "--silent") { encparms.silent = 1; } else { break; } - - ++_arguments_parsed; } return true; } bool CompressionTool::processOggParms() { - while (_arguments_parsed < _arguments.size()) { - std::string arg = _arguments[_arguments_parsed]; + while (!_arguments.empty()) { + std::string arg = _arguments.front(); + _arguments.pop_front(); if (arg == "-b") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -b"); - oggparms.nominalBitr = atoi(_arguments[_arguments_parsed].c_str()); + oggparms.nominalBitr = atoi(_arguments.front().c_str()); if ((oggparms.nominalBitr % 8) != 0) oggparms.nominalBitr -= oggparms.nominalBitr % 8; @@ -763,17 +766,18 @@ bool CompressionTool::processOggParms() { if (oggparms.nominalBitr > 160) throw ToolException("Nominal bitrate out of bounds (-b), must be between 8 and 160."); - if (oggparms.nominalBitr == 0 && _arguments[_arguments_parsed] != "0") + if (oggparms.nominalBitr == 0 && _arguments.front() != "0") throw ToolException("Nominal bitrate (-b) must be a number."); if (oggparms.nominalBitr < 8) throw ToolException("Nominal bitrate out of bounds (-b), must be between 8 and 160."); + _arguments.pop_front(); + } else if (arg == "-m") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -m"); - oggparms.minBitr = atoi(_arguments[_arguments_parsed].c_str()); + oggparms.minBitr = atoi(_arguments.front().c_str()); if ((oggparms.minBitr % 8) != 0) oggparms.minBitr -= oggparms.minBitr % 8; @@ -781,17 +785,18 @@ bool CompressionTool::processOggParms() { if (oggparms.minBitr > 160) throw ToolException("Minimal bitrate out of bounds (-m), must be between 8 and 160."); - if (oggparms.minBitr == 0 && _arguments[_arguments_parsed] != "0") + if (oggparms.minBitr == 0 && _arguments.front() != "0") throw ToolException("Minimal bitrate (-m) must be a number."); if (oggparms.minBitr < 8) throw ToolException("Minimal bitrate out of bounds (-m), must be between 8 and 160."); + _arguments.pop_front(); + } else if (arg == "-M") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -M"); - oggparms.maxBitr = atoi(_arguments[_arguments_parsed].c_str()); + oggparms.maxBitr = atoi(_arguments.front().c_str()); if ((oggparms.maxBitr % 8) != 0) oggparms.maxBitr -= oggparms.maxBitr % 8; @@ -799,38 +804,42 @@ bool CompressionTool::processOggParms() { if (oggparms.maxBitr > 160) throw ToolException("Minimal bitrate out of bounds (-M), must be between 8 and 160."); - if (oggparms.maxBitr == 0 && _arguments[_arguments_parsed] != "0") + if (oggparms.maxBitr == 0 && _arguments.front() != "0") throw ToolException("Minimal bitrate (-M) must be a number."); if (oggparms.maxBitr < 8) throw ToolException("Minimal bitrate out of bounds (-M), must be between 8 and 160."); - } else if (arg == "-q") { - ++_arguments_parsed; - oggparms.quality = (float)atoi(_arguments[_arguments_parsed].c_str()); - if (oggparms.quality == 0 && _arguments[_arguments_parsed] != "0") + _arguments.pop_front(); + + } else if (arg == "-q") { + oggparms.quality = (float)atoi(_arguments.front().c_str()); + + if (oggparms.quality == 0 && _arguments.front() != "0") throw ToolException("Quality (-q) must be a number."); + + _arguments.pop_front(); + } else if (arg == "--silent") { oggparms.silent = 1; } else { break; } - - ++_arguments_parsed; } return true; } bool CompressionTool::processFlacParms(){ - while (_arguments_parsed < _arguments.size()) { - std::string arg = _arguments[_arguments_parsed]; + while (!_arguments.empty()) { + std::string arg = _arguments.front(); + _arguments.pop_front(); if (arg == "-b") { - ++_arguments_parsed; - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) throw ToolException("Could not parse command line options, expected value after -b"); - flacparms.blocksize = atoi(_arguments[_arguments_parsed].c_str()); + flacparms.blocksize = atoi(_arguments.front().c_str()); + _arguments.pop_front(); } else if (arg == "--fast") { flacparms.compressionLevel = 0; } else if (arg == "--best") { @@ -860,8 +869,6 @@ bool CompressionTool::processFlacParms(){ } else { break; } - - ++_arguments_parsed; } return true; @@ -878,17 +885,17 @@ CompressionTool::CompressionTool(const std::string &name, ToolType type) : Tool( void CompressionTool::parseAudioArguments() { _format = AUDIO_MP3; - if (_arguments[_arguments_parsed] == "--mp3") + if (_arguments.front() == "--mp3") _format = AUDIO_MP3; - else if (_arguments[_arguments_parsed] == "--vorbis") + else if (_arguments.front() == "--vorbis") _format = AUDIO_VORBIS; - else if (_arguments[_arguments_parsed] == "--flac") + else if (_arguments.front() == "--flac") _format = AUDIO_FLAC; else // No audio arguments then return; - ++_arguments_parsed; + _arguments.pop_front(); // Need workaround to be sign-correct switch (_format) { diff --git a/compress_agos.cpp b/compress_agos.cpp index eb55fd7d..053dbc07 100644 --- a/compress_agos.cpp +++ b/compress_agos.cpp @@ -214,9 +214,9 @@ void CompressAgos::convert_mac(Filename *inputPath) { } void CompressAgos::parseExtraArguments() { - if (_arguments[_arguments_parsed] == "--mac") { + if (!_arguments.empty() && _arguments.front() == "--mac") { _convertMac = true; - ++_arguments_parsed; + _arguments.pop_front(); } } diff --git a/compress_gob.cpp b/compress_gob.cpp index c5763a65..663b8dd0 100644 --- a/compress_gob.cpp +++ b/compress_gob.cpp @@ -58,9 +58,9 @@ CompressGob::~CompressGob() { } void CompressGob::parseExtraArguments() { - if (_arguments[_arguments_parsed] == "-f") { + if (!_arguments.empty() && _arguments.front() == "-f") { _execMode |= MODE_FORCE; - ++_arguments_parsed; + _arguments.pop_front(); } } diff --git a/compress_sword1.cpp b/compress_sword1.cpp index 91e112d5..75d7ee17 100644 --- a/compress_sword1.cpp +++ b/compress_sword1.cpp @@ -581,13 +581,13 @@ CompressSword1::CompressSword1(const std::string &name) : CompressionTool(name, } void CompressSword1::parseExtraArguments() { - if (_arguments[_arguments_parsed] == "--speech-only") { + if (!_arguments.empty() && _arguments.front() == "--speech-only") { _compMusic = false; - ++_arguments_parsed; + _arguments.pop_front(); } - if (_arguments[_arguments_parsed] == "--music-only") { + if (!_arguments.empty() && _arguments.front() == "--music-only") { _compSpeech = false; - ++_arguments_parsed; + _arguments.pop_front(); } } diff --git a/example_tool.cpp b/example_tool.cpp index e80cd6e8..6ed3c351 100644 --- a/example_tool.cpp +++ b/example_tool.cpp @@ -91,15 +91,14 @@ CompressionExample::CompressionExample(const std::string &name) : Tool(name, TOO void CompressionExample::parseExtraArguments() { // Here, we parse our own arguments - // Arguments are stored in the _arguments member - // and the number of arguments already parsed is in _ - if (_arguments.size() < _arguments_parsed) { - if (_arguments[_arguments_parsed] == "-a") { + // Arguments are stored in the _arguments member. + // Remove any arguments that you have "used up" + if (!_arguments.empty() && _arguments.front() == "-a") { // Set our member _outputFiles = true; - // We matched one argument, make sure to advance argument counter - ++_arguments_parsed; + // We matched one argument, so remove it + _arguments.pop_front(); } } } diff --git a/extract_kyra.cpp b/extract_kyra.cpp index 1c9a0363..c39b91b6 100644 --- a/extract_kyra.cpp +++ b/extract_kyra.cpp @@ -52,8 +52,9 @@ ExtractKyra::ExtractKyra(const std::string &name) : Tool(name, TOOLTYPE_EXTRACTI void ExtractKyra::parseExtraArguments() { // Parse our own arguments - while(_arguments_parsed < _arguments.size()) { - std::string arg = _arguments[_arguments_parsed]; + while (!_arguments.empty()) { + std::string arg = _arguments.front(); + _arguments.pop_front(); if (arg == "-x") { extractAll = true; extractOne = false; @@ -65,17 +66,13 @@ void ExtractKyra::parseExtraArguments() { extractOne = true; extractAll = false; - ++_arguments_parsed; - - if (_arguments_parsed >= _arguments.size()) { + if (_arguments.empty()) error("No filename supplied to -n\nShould be used on the form: %s -n ALGAE.CPS -o out/ A_E.PAK"); - } else { - singleFilename = _arguments[_arguments_parsed]; - } + singleFilename = _arguments.front(); + _arguments.pop_front(); } else { break; } - ++_arguments_parsed; } } diff --git a/extract_parallaction.cpp b/extract_parallaction.cpp index 6f3cdad2..7e87d37b 100644 --- a/extract_parallaction.cpp +++ b/extract_parallaction.cpp @@ -305,9 +305,9 @@ ExtractParallaction::ExtractParallaction(const std::string &name) : Tool(name, T } void ExtractParallaction::parseExtraArguments() { - if (_arguments[_arguments_parsed] == "--small") { + if (!_arguments.empty() && _arguments.front() == "--small") { _small = true; - ++_arguments_parsed; + _arguments.pop_front(); } } diff --git a/tool.cpp b/tool.cpp index c9568e5c..81a142c3 100644 --- a/tool.cpp +++ b/tool.cpp @@ -32,8 +32,6 @@ Tool::Tool(const std::string &name, ToolType type) { _name = name; _type = type; - _arguments_parsed = 0; - _outputToDirectory = true; _supportedFormats = AUDIO_ALL; _supportsProgressBar = false; @@ -56,26 +54,14 @@ Tool::~Tool() { // ... } -int Tool::run(int argc, char *argv[]) { - argc -= 1; - argv += 1; - - std::vector args; - for (int i = 0; i < argc; ++i) - args.push_back(argv[i]); - - return run(args); -} - -int Tool::run(std::vector args) { +int Tool::run(const std::deque &args) { _arguments = args; - _arguments_parsed = 0; // Pop the first argument (name of ourselves) - _arguments.erase(_arguments.begin()); + _arguments.pop_front(); // Check for help - if (_arguments.empty() || _arguments[0] == "-h" || _arguments[0] == "--help") { + if (_arguments.empty() || _arguments.front() == "-h" || _arguments.front() == "--help") { print(getHelp().c_str()); return 2; } @@ -87,20 +73,21 @@ int Tool::run(std::vector args) { // Read tool specific arguments parseExtraArguments(); - if (_arguments.size() && _arguments[_arguments_parsed][0] == '-') { - std::string s = "Possibly ignored option " + _arguments[_arguments_parsed] + "."; + if (!_arguments.empty() && _arguments.front()[0] == '-') { + std::string s = "Possibly ignored option " + _arguments.front() + "."; print(s.c_str()); } // Make sure we have enough input files. - if (_arguments.size() - _arguments_parsed < _inputPaths.size()) { + if (_arguments.size() < _inputPaths.size()) { print("Too few input files!"); return -2; } // Read input files from CLI for (ToolInputs::iterator iter = _inputPaths.begin(); iter != _inputPaths.end(); ++iter) { - std::string &in = _arguments[_arguments_parsed++]; + std::string &in = _arguments.front(); + _arguments.pop_front(); if (!iter->file) { // Append '/' to input if it's not already done // TODO: We need a way to detect a proper directory here! @@ -115,11 +102,13 @@ int Tool::run(std::vector args) { } // We should have parsed all arguments by now - if (_arguments_parsed < _arguments.size() - _inputPaths.size()) { + if (_inputPaths.size() < _arguments.size()) { std::ostringstream os; os << "Too many inputs files ( "; - while (_arguments_parsed < _arguments.size()) - os << "'" << _arguments[_arguments_parsed++] << "' "; + while (!_arguments.empty()) { + os << "'" << _arguments.front() << "' "; + _arguments.pop_front(); + } os << ")\n"; print(os.str().c_str()); return -2; @@ -272,17 +261,16 @@ void Tool::parseAudioArguments() { } void Tool::parseOutputArguments() { - if (_arguments_parsed >= _arguments.size()) + if (_arguments.empty()) return; - if (_arguments[_arguments_parsed] == "-o" || _arguments[_arguments_parsed] == "--output") { + if (_arguments.front() == "-o" || _arguments.front() == "--output") { // It's an -o argument - if (_arguments_parsed + 1 < _arguments.size()) { - _outputPath = _arguments[_arguments_parsed + 1]; - _arguments_parsed += 2; - } else { + _arguments.pop_front(); + if (_arguments.empty()); throw ToolException("Could not parse arguments: Expected path after '-o' or '--output'."); - } + + _outputPath = _arguments.front(); } } diff --git a/tool.h b/tool.h index 5186e9b1..8b0c6e88 100644 --- a/tool.h +++ b/tool.h @@ -24,6 +24,7 @@ #define TOOL_H #include +#include #include #include "util.h" @@ -78,15 +79,9 @@ public: virtual ~Tool(); /** - * Run tool with CLI args (parses them, and then calls run()) - * This version also catches all errors and prints them before exiting - * - * @param argc Argument count - * @param argv Argument values + * Run tool with command line arguments. */ - int run(int argc, char *argv[]); - /** Same as the above, but accepts vector of string instead */ - int run(std::vector args); + int run(const std::deque &args); /** * Parse with args set already (modify the public members to set them) @@ -211,9 +206,7 @@ public: protected: /* Command line arguments we are parsing. */ - std::vector _arguments; - /* How many of the arguments we have parsed so far */ - size_t _arguments_parsed; + std::deque _arguments; /** If this tools outputs to a directory, not a file. */ bool _outputToDirectory; diff --git a/tools_cli.cpp b/tools_cli.cpp index 94eb9520..4bb56e9b 100644 --- a/tools_cli.cpp +++ b/tools_cli.cpp @@ -57,7 +57,7 @@ int ToolsCLI::run(int argc, char *argv[]) { Tool *tool = *iter; if (arguments.front() == tool->getName()) { // Run the tool, first argument will be name, very nice! - return tool->run(std::vector(arguments.begin(), arguments.end())); + return tool->run(arguments); } } std::cout << "\tUnknown tool, make sure you input one of the following:\n"; @@ -69,7 +69,7 @@ int ToolsCLI::run(int argc, char *argv[]) { for (ToolList::iterator iter = _tools.begin(); iter != _tools.end(); ++iter) { Tool *tool = *iter; if (arguments.front() == tool->getName()) { - // Run the tool, first argument will be name, very nice! + // Obtain the help text for this tool and print it std::cout << tool->getHelp() << std::endl; return 2; } @@ -179,7 +179,7 @@ int ToolsCLI::run(int argc, char *argv[]) { // Run the tool, with the remaining arguments arguments.push_front(tool->getName()); - return tool->run(std::vector(arguments.begin(), arguments.end())); + return tool->run(arguments); } return 0;