From 8b63985ac986d64dbd3f210e5b265a4feffe7121 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sat, 21 Oct 2023 19:45:07 -0700 Subject: [PATCH] Fix flags usage and make Commander Flags capable. - Programs should exit on Flags parse error. - Commander now accepts a string vector for interop with Flags. --- .circleci/config.yml | 7 ++- include/scmp/geom/Quaternion.h | 97 +++++++++++++++++----------------- include/scsl/Commander.h | 15 +++--- src/bin/phonebook.cc | 81 ++++++++++++++++------------ src/sl/Commander.cc | 13 ++--- src/sl/Flags.cc | 2 +- test/buffer.cc | 1 + test/coord2d.cc | 1 + test/dictionary.cc | 1 + test/madgwick.cc | 1 + test/math.cc | 1 + test/orientation.cc | 1 + test/quaternion.cc | 1 + test/simple_suite_example.cc | 1 + test/stringutil.cc | 1 + test/tlv.cc | 1 + test/vector.cc | 1 + 17 files changed, 127 insertions(+), 99 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8d58979..72607c8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -9,7 +9,10 @@ jobs: - run: name: Setup cmake build command: cmake-build-and-test.sh - static_analysis: + - run: + name: Valgrind checks. + command: cmake-run-valgrind.sh + NOT_REQUIRED_static_analysis: docker: - image: git.wntrmute.dev/sc/dev:main steps: @@ -24,4 +27,4 @@ workflows: - ctest static_analysis: jobs: - - static_analysis + - NOT_REQUIRED_static_analysis diff --git a/include/scmp/geom/Quaternion.h b/include/scmp/geom/Quaternion.h index d5d0211..40557d7 100644 --- a/include/scmp/geom/Quaternion.h +++ b/include/scmp/geom/Quaternion.h @@ -60,7 +60,7 @@ namespace geom { template class Quaternion { public: - /// \brief Construct an identity MakeQuaternion. + /// \brief Construct an identity Quaternion. Quaternion() : v(Vector{0.0, 0.0, 0.0}), w(1.0) { scmp::DefaultEpsilon(this->eps); @@ -68,7 +68,7 @@ public: }; - /// \brief Construct a MakeQuaternion with an Axis and Angle of + /// \brief Construct a Quaternion with an Axis and Angle of /// rotation. /// /// A Quaternion may be initialised with a Vector Axis @@ -119,7 +119,7 @@ public: } - /// \brief Set the comparison tolerance for this MakeQuaternion. + /// \brief Set the comparison tolerance for this Quaternion. /// /// \param epsilon A tolerance value. void @@ -130,9 +130,9 @@ public: } - /// \brief Return the Axis of rotation of this MakeQuaternion. + /// \brief Return the Axis of rotation of this Quaternion. /// - /// \return The Axis of rotation of this MakeQuaternion. + /// \return The Axis of rotation of this Quaternion. Vector Axis() const { @@ -140,9 +140,9 @@ public: } - /// \brief Return the Angle of rotation of this MakeQuaternion. + /// \brief Return the Angle of rotation of this Quaternion. /// - /// \return the Angle of rotation of this MakeQuaternion. + /// \return the Angle of rotation of this Quaternion. T Angle() const { @@ -152,7 +152,7 @@ public: /// \brief Compute the Dot product of two quaternions. /// - /// \param other Another MakeQuaternion. + /// \param other Another Quaternion. /// \return The Dot product between the two quaternions. T Dot(const Quaternion &other) const @@ -166,7 +166,7 @@ public: } - /// \brief Compute the Norm of a MakeQuaternion. + /// \brief Compute the Norm of a Quaternion. /// /// Treating the Quaternion as a Vector, this is the same /// process as computing the Magnitude. @@ -186,18 +186,18 @@ public: } - /// \brief Return the unit MakeQuaternion. + /// \brief Return the unit Quaternion. /// - /// \return The unit MakeQuaternion. + /// \return The unit Quaternion. Quaternion UnitQuaternion() { return *this / this->Norm(); } - /// \brief Compute the Conjugate of a MakeQuaternion. + /// \brief Compute the Conjugate of a Quaternion. /// - /// \return The Conjugate of this MakeQuaternion. + /// \return The Conjugate of this Quaternion. Quaternion Conjugate() const { @@ -205,9 +205,9 @@ public: } - /// \brief Compute the Inverse of a MakeQuaternion. + /// \brief Compute the Inverse of a Quaternion. /// - /// \return The Inverse of this MakeQuaternion. + /// \return The Inverse of this Quaternion. Quaternion Inverse() const { @@ -217,9 +217,9 @@ public: } - /// \brief Determine whether this is an identity MakeQuaternion. + /// \brief Determine whether this is an identity Quaternion. /// - /// \return true if this is an identity MakeQuaternion. + /// \return true if this is an identity Quaternion. bool IsIdentity() const { return this->v.IsZero() && @@ -227,22 +227,23 @@ public: } - /// \brief Determine whether this is a unit MakeQuaternion. + /// \brief Determine whether this is a unit Quaternion. /// - /// \return true if this is a unit MakeQuaternion. + /// \return true if this is a unit Quaternion. bool IsUnitQuaternion() const { - return scmp::WithinTolerance(this->Norm(), (T) 1.0, this->eps); + auto normal = this->Norm(); + return scmp::WithinTolerance(normal, (T) 1.0, this->eps); } /// \brief Convert to Vector form. /// - /// Return the MakeQuaternion as a Vector, with the Axis of + /// Return the Quaternion as a Vector, with the Axis of /// rotation followed by the Angle of rotation. /// - /// \return A vector representation of the MakeQuaternion. + /// \return A vector representation of the Quaternion. Vector AsVector() const { @@ -250,7 +251,7 @@ public: } - /// \brief Rotate Vector vr about this MakeQuaternion. + /// \brief Rotate Vector vr about this Quaternion. /// /// \param vr The vector to be rotated. /// \return The rotated vector. @@ -261,9 +262,9 @@ public: } - /// \brief Return Euler angles for this MakeQuaternion. + /// \brief Return Euler angles for this Quaternion. /// - /// Return the Euler angles for this MakeQuaternion as a vector of + /// Return the Euler angles for this Quaternion as a vector of /// . /// /// \warning Users of this function should watch out for gimbal @@ -289,7 +290,7 @@ public: /// \brief Quaternion addition. /// - /// \param other The MakeQuaternion to be added with this one. + /// \param other The Quaternion to be added with this one. /// \return The result of adding the two quaternions together. Quaternion operator+(const Quaternion &other) const @@ -300,8 +301,8 @@ public: /// \brief Quaternion subtraction. /// - /// \param other The MakeQuaternion to be subtracted from this one. - /// \return The result of subtracting the other MakeQuaternion from this one. + /// \param other The Quaternion to be subtracted from this one. + /// \return The result of subtracting the other Quaternion from this one. Quaternion operator-(const Quaternion &other) const { @@ -312,7 +313,7 @@ public: /// \brief Scalar multiplication. /// /// \param k The scaling value. - /// \return A scaled MakeQuaternion. + /// \return A scaled Quaternion. Quaternion operator*(const T k) const { @@ -323,7 +324,7 @@ public: /// \brief Scalar division. /// /// \param k The scalar divisor. - /// \return A scaled MakeQuaternion. + /// \return A scaled Quaternion. Quaternion operator/(const T k) const { @@ -334,11 +335,11 @@ public: /// \brief Quaternion Hamilton multiplication with a three- /// dimensional vector. /// - /// This is done by treating the vector as a pure MakeQuaternion + /// This is done by treating the vector as a pure Quaternion /// (e.g. with an Angle of rotation of 0). /// - /// \param vector The vector to multiply with this MakeQuaternion. - /// \return The Hamilton product of the MakeQuaternion and vector. + /// \param vector The vector to multiply with this Quaternion. + /// \return The Hamilton product of the Quaternion and vector. Quaternion operator*(const Vector &vector) const { @@ -349,7 +350,7 @@ public: /// \brief Quaternion Hamilton multiplication. /// - /// \param other The other MakeQuaternion to multiply with this one. + /// \param other The other Quaternion to multiply with this one. /// @result The Hamilton product of the two quaternions. Quaternion operator*(const Quaternion &other) const @@ -365,7 +366,7 @@ public: /// \brief Quaternion equivalence. /// - /// \param other The MakeQuaternion to check equality against. + /// \param other The Quaternion to check equality against. /// \return True if the two quaternions are equal within their tolerance. bool operator==(const Quaternion &other) const @@ -377,7 +378,7 @@ public: /// \brief Quaternion non-equivalence. /// - /// \param other The MakeQuaternion to check inequality against. + /// \param other The Quaternion to check inequality against. /// \return True if the two quaternions are unequal within their tolerance. bool operator!=(const Quaternion &other) const @@ -386,13 +387,13 @@ public: } - /// \brief Output a MakeQuaternion to a stream in the form + /// \brief Output a Quaternion to a stream in the form /// `a + `. /// /// \todo improve the formatting. /// /// \param outs An output stream - /// \param q A MakeQuaternion + /// \param q A Quaternion /// \return The output stream friend std::ostream & operator<<(std::ostream &outs, const Quaternion &q) @@ -438,41 +439,41 @@ typedef Quaternion Quaterniond; /// \brief Convenience Quaternion construction function. /// -/// Return a float MakeQuaternion scaled appropriately from a vector and +/// Return a float Quaternion scaled appropriately from a vector and /// Angle, e.g. /// angle = cos(Angle / 2), /// Axis.UnitVector() * sin(Angle / 2). /// /// \param axis The Axis of rotation. /// \param angle The Angle of rotation. -/// \return A MakeQuaternion. +/// \return A Quaternion. /// \relatesalso Quaternion Quaternionf MakeQuaternion(Vector3F axis, float angle); /// \brief Convience Quaternion construction function. /// -/// Return a double MakeQuaternion scaled appropriately from a vector and +/// Return a double Quaternion scaled appropriately from a vector and /// Angle, e.g. /// Angle = cos(Angle / 2), /// Axis.UnitVector() * sin(Angle / 2). /// /// \param axis The Axis of rotation. /// \param angle The Angle of rotation. -/// \return A MakeQuaternion. +/// \return A Quaternion. /// \relatesalso Quaternion Quaterniond MakeQuaternion(Vector3D axis, double angle); /// \brief Convience Quaternion construction function. /// -/// Return a double MakeQuaternion scaled appropriately from a vector and +/// Return a double Quaternion scaled appropriately from a vector and /// Angle, e.g. /// Angle = cos(Angle / 2), /// Axis.UnitVector() * sin(Angle / 2). /// /// \param axis The Axis of rotation. /// \param angle The Angle of rotation. -/// \return A MakeQuaternion. +/// \return A Quaternion. /// \relatesalso Quaternion template Quaternion @@ -513,8 +514,8 @@ Quaterniond DoubleQuaternionFromEuler(Vector3D euler); /// fraction of the distance between them. /// /// \tparam T -/// \param p The starting MakeQuaternion. -/// \param q The ending MakeQuaternion. +/// \param p The starting Quaternion. +/// \param q The ending Quaternion. /// \param t The fraction of the distance between the two quaternions to /// interpolate. /// \return A Quaternion representing the linear interpolation of the @@ -534,8 +535,8 @@ LERP(Quaternion p, Quaternion q, T t) /// distance between them. /// /// \tparam T -/// \param p The starting MakeQuaternion. -/// \param q The ending MakeQuaternion.Short +/// \param p The starting Quaternion. +/// \param q The ending Quaternion. /// \param t The fraction of the distance between the two quaternions /// to interpolate. /// \return A Quaternion representing the shortest path between two diff --git a/include/scsl/Commander.h b/include/scsl/Commander.h index 9707c4e..2956640 100644 --- a/include/scsl/Commander.h +++ b/include/scsl/Commander.h @@ -44,7 +44,7 @@ namespace scsl { /// CommanderFunc describes a function that can be run in Commander. /// /// It expects an argument count and a list of arguments. -using CommanderFunc = std::function; +using CommanderFunc = std::function)>; /// \brief Subcommands used by Commander. @@ -72,8 +72,8 @@ public: /// \param name The subcommand name; this is the name that will select this command. /// \param argc The minimum number of arguments required by this subcommand. /// \param func A valid CommanderFunc. - Subcommand(std::string name, int argc, CommanderFunc func) - : fn(func), args(argc), command(name) + Subcommand(std::string name, size_t argc, CommanderFunc func) + : fn(func), requiredArgs(argc), command(name) {} /// Name returns the name of this subcommand. @@ -81,13 +81,12 @@ public: /// Run attempts to run the CommanderFunc for this subcommand. /// - /// \param argc The number of arguments supplied. - /// \param argv The argument list. + /// \param args The argument list. /// \return A Status type indicating the status of running the command. - Status Run(int argc, char **argv); + Status Run(std::vector args); private: CommanderFunc fn; - int args; + size_t requiredArgs; std::string command; }; @@ -119,7 +118,7 @@ public: bool Register(Subcommand scmd); /// Try to run a subcommand registered with this Commander. - Subcommand::Status Run(std::string command, int argc, char **argv); + Subcommand::Status Run(std::string command, std::vector args); private: std::map cmap; }; diff --git a/src/bin/phonebook.cc b/src/bin/phonebook.cc index 53e4336..ae9e976 100644 --- a/src/bin/phonebook.cc +++ b/src/bin/phonebook.cc @@ -27,48 +27,50 @@ using namespace std; #include #include #include +#include + + using namespace scsl; static const char *defaultPhonebook = "pb.dat"; -static char *pbFile = (char *)defaultPhonebook; +static std::string pbFile(defaultPhonebook); static Arena arena; static Dictionary pb(arena); static bool -listFiles(int argc, char **argv) +listFiles(std::vector argv) { - (void)argc; (void)argv; + (void) argv; // provided for interface compatibility. cout << "[+] keys in '" << pbFile << "':\n"; cout << pb; return true; } -static bool -newPhonebook(int argc, char **argv) -{ - (void)argc; - auto size = std::stoul(string(argv[0])); +static bool +newPhonebook(std::vector argv) +{ + auto size = std::stoul(argv[0]); cout << "[+] create new " << size << "B phonebook '" << pbFile << "'\n"; - return arena.Create(pbFile, size) == 0; + return arena.Create(pbFile.c_str(), size) == 0; } + static bool -delKey(int argc, char **argv) +delKey(std::vector argv) { - (void)argc; string key = argv[0]; cout << "[+] deleting key '" << key << "'\n"; return pb.Delete(key.c_str(), key.size()); } + static bool -hasKey(int argc, char **argv) +hasKey(std::vector argv) { - (void)argc; string key = argv[0]; cout << "[+] looking up '" << key << "': "; @@ -81,10 +83,10 @@ hasKey(int argc, char **argv) return true; } + static bool -getKey(int argc, char **argv) +getKey(std::vector argv) { - (void)argc; TLV::Record rec; auto key = string(argv[0]); @@ -100,9 +102,8 @@ getKey(int argc, char **argv) static bool -putKey(int argc, char **argv) +putKey(std::vector argv) { - (void)argc; auto key = string(argv[0]); auto val = string(argv[1]); @@ -116,6 +117,7 @@ putKey(int argc, char **argv) return true; } + static void usage(ostream &os, int exc) { @@ -136,28 +138,35 @@ usage(ostream &os, int exc) int main(int argc, char *argv[]) { - int optind = 1; + bool help = false; + std::string fileName(pbFile); - for (optind = 1; optind < argc; optind++) { - auto arg = string(argv[optind]); - if (arg[0] != '-') break; - if (arg == "-h") usage(cout, 0); - if (arg == "-f") { - pbFile = argv[optind+1]; - optind++; - continue; - } + auto *flags = new scsl::Flags("phonebook", + "A tool for interacting with Arena-backed dictionary files."); + flags->Register("-f", pbFile, "path to a phonebook file"); + flags->Register("-h", false, "print a help message"); - usage(cerr, 1); + auto parsed = flags->Parse(argc, argv); + if (parsed != scsl::Flags::ParseStatus::OK) { + std::cerr << "Failed to parse flags: " + << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } - if (argc <= 1) { - usage(cout, 0); + flags->GetString("-f", fileName); + flags->GetBool("-h", help); + + pbFile = fileName; + + if (help) { + usage(std::cerr, 1); + } + + if (flags->NumArgs() == 0) { + usage(std::cerr, 1); } - auto command = string(argv[optind++]); Commander commander; - commander.Register(Subcommand("list", 0, listFiles)); commander.Register(Subcommand("new", 1, newPhonebook)); commander.Register(Subcommand("del", 1, delKey)); @@ -165,15 +174,19 @@ main(int argc, char *argv[]) commander.Register(Subcommand("get", 1, getKey)); commander.Register(Subcommand("put", 2, putKey)); + auto command = flags->Arg(0); if (command != "new") { cout << "[+] loading phonebook from " << pbFile << "\n"; - if (arena.Open(pbFile) != 0) { + if (arena.Open(pbFile.c_str()) != 0) { cerr << "Failed to open " << pbFile << "\n"; exit(1); } } - auto result = commander.Run(command, argc-optind, argv+optind); + auto args = flags->Args(); + args.erase(args.begin()); + + auto result = commander.Run(command, args); switch (result) { case Subcommand::Status::OK: std::cout << "[+] OK\n"; diff --git a/src/sl/Commander.cc b/src/sl/Commander.cc index 812f122..4f1a1ac 100644 --- a/src/sl/Commander.cc +++ b/src/sl/Commander.cc @@ -29,15 +29,16 @@ namespace scsl { Subcommand::Status -Subcommand::Run(int argc, char **argv) +Subcommand::Run(std::vector args) { - if (argc < this->args) { + auto argc = args.size(); + if (argc < this->requiredArgs) { std::cerr << "[!] " << this->command << " expects "; - std::cerr << this->args << " args, but was given "; + std::cerr << this->requiredArgs << " args, but was given "; std::cerr << argc << " args.\n"; return Subcommand::Status::NotEnoughArgs; } - if (this->fn(argc, argv)) { + if (this->fn(args)) { return Subcommand::Status::OK; } @@ -64,14 +65,14 @@ Commander::Register(Subcommand scmd) Subcommand::Status -Commander::Run(std::string command, int argc, char **argv) +Commander::Run(std::string command, std::vector args) { if (this->cmap.count(command) != 1) { return Subcommand::Status::CommandNotRegistered; } auto scmd = this->cmap[command]; - return scmd->Run(argc, argv); + return scmd->Run(args); } diff --git a/src/sl/Flags.cc b/src/sl/Flags.cc index 7e804ef..15d22c6 100644 --- a/src/sl/Flags.cc +++ b/src/sl/Flags.cc @@ -95,7 +95,7 @@ Flags::~Flags() bool Flags::Register(std::string fName, FlagType fType, std::string fDescription) { - if (!std::regex_search(fName, isFlag) || fName == "-h") { + if (!std::regex_search(fName, isFlag)) { return false; } diff --git a/test/buffer.cc b/test/buffer.cc index c51ab4e..a1a5270 100644 --- a/test/buffer.cc +++ b/test/buffer.cc @@ -90,6 +90,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } sctest::SimpleSuite suite; diff --git a/test/coord2d.cc b/test/coord2d.cc index b744906..2405e53 100755 --- a/test/coord2d.cc +++ b/test/coord2d.cc @@ -270,6 +270,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } SimpleSuite suite; diff --git a/test/dictionary.cc b/test/dictionary.cc index 8101420..fdff67b 100644 --- a/test/dictionary.cc +++ b/test/dictionary.cc @@ -122,6 +122,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } sctest::SimpleSuite suite; diff --git a/test/madgwick.cc b/test/madgwick.cc index 55ae887..c49fe03 100644 --- a/test/madgwick.cc +++ b/test/madgwick.cc @@ -245,6 +245,7 @@ main(int argc, char **argv) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } sctest::SimpleSuite suite; diff --git a/test/math.cc b/test/math.cc index 6928ffe..44710e2 100644 --- a/test/math.cc +++ b/test/math.cc @@ -133,6 +133,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } sctest::SimpleSuite suite; diff --git a/test/orientation.cc b/test/orientation.cc index 20759dc..ef74618 100644 --- a/test/orientation.cc +++ b/test/orientation.cc @@ -99,6 +99,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } SimpleSuite suite; diff --git a/test/quaternion.cc b/test/quaternion.cc index c9557f3..9cb1d1e 100644 --- a/test/quaternion.cc +++ b/test/quaternion.cc @@ -442,6 +442,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } SimpleSuite suite; diff --git a/test/simple_suite_example.cc b/test/simple_suite_example.cc index 2da4c15..da0da9e 100755 --- a/test/simple_suite_example.cc +++ b/test/simple_suite_example.cc @@ -66,6 +66,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } sctest::SimpleSuite suite; diff --git a/test/stringutil.cc b/test/stringutil.cc index 8228ac6..954a820 100644 --- a/test/stringutil.cc +++ b/test/stringutil.cc @@ -149,6 +149,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } sctest::SimpleSuite suite; diff --git a/test/tlv.cc b/test/tlv.cc index 520ece7..8a01fad 100644 --- a/test/tlv.cc +++ b/test/tlv.cc @@ -151,6 +151,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } sctest::SimpleSuite suite; diff --git a/test/vector.cc b/test/vector.cc index 7d5a033..3e423fe 100644 --- a/test/vector.cc +++ b/test/vector.cc @@ -442,6 +442,7 @@ main(int argc, char *argv[]) if (parsed != scsl::Flags::ParseStatus::OK) { std::cerr << "Failed to parse flags: " << scsl::Flags::ParseStatusToString(parsed) << "\n"; + exit(1); } SimpleSuite suite;