From 9a8dc08a4fbe9c0462c8b2c8b1a2da48b923e813 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sat, 21 Oct 2023 23:33:22 -0700 Subject: [PATCH] cppcheck cleanups and additional docs+testing. --- include/scsl/Buffer.h | 99 ++++++++++++++++++++++++------------------- src/bin/phonebook.cc | 6 +-- src/sl/Arena.cc | 25 ++++------- src/sl/Buffer.cc | 67 +++++++++++++++++++++++++++-- src/sl/Commander.cc | 3 +- src/sl/Flags.cc | 2 +- test/buffer.cc | 64 +++++++++++++++++++++++++--- test/dictionary.cc | 1 + 8 files changed, 191 insertions(+), 76 deletions(-) diff --git a/include/scsl/Buffer.h b/include/scsl/Buffer.h index 7f1b370..83b520b 100644 --- a/include/scsl/Buffer.h +++ b/include/scsl/Buffer.h @@ -47,99 +47,116 @@ namespace scsl { /// memory if possible, but only if #AutoTrimIsEnabled (it is by default). class Buffer { public: - /// A Buffer can be constructed empty, with no memory allocated (yet). + /// \brief Construct an empty buffer with no memory allocated. Buffer(); - /// A Buffer can be constructed with an explicit capacity. + /// \buffer Constructor with explicit memory capacity. /// - /// \param initialCapacity The initial allocation size for the buffer. + /// \param initialCapacity The initial allocation size for the + /// buffer. explicit Buffer(size_t initialCapacity); - /// A Buffer can be initialized with a starting C-style string. + /// \brief Construct with a C-style string. explicit Buffer(const char *s); - /// A Buffer can be initialized with a starting string. - explicit Buffer(const std::string s); + /// \buffer Construct with an initial string. + explicit Buffer(const std::string& s); - ~Buffer() - { this->Reclaim(); } + ~Buffer(); - /// Contents returns the Buffer's contents. - uint8_t *Contents() const - { return this->contents; } + /// \brief Retrieve the buffer's contents. + uint8_t *Contents() const; - /// Length returns the length of the data currently stored in the - /// buffer. - size_t Length() const - { return this->length; }; + std::string ToString() const; - /// Capacity returns the amount of memory allocated to the Buffer. - size_t Capacity() const - { return this->capacity; } + /// \brief The length of data stored in the buffer. + /// + /// \return The number of bytes stored in the Buffer. + size_t Length() const; - /// Append copies in a C-style string to the end of the buffer. + /// \brief Return the amount of memory allocated for the + /// Buffer. + size_t Capacity() const; + + /// \brief Append a C-style string to the end of the buffer. /// /// \param s The string to append. /// \return True if the Buffer was resized. bool Append(const char *s); - /// Append copies in a string to the end of the buffer. + /// Append Append a string to the end of the buffer. /// /// \param s The string to append. /// \return True if the Buffer was resized. - bool Append(const std::string s); + bool Append(const std::string &s); - /// Append copies in a byte buffer to the end of the buffer. + /// \brief Append a byte buffer to the end of the buffer. /// /// \param data The byte buffer to insert. /// \param datalen The length of the byte buffer. /// \return True if the Buffer was resized. bool Append(const uint8_t *data, const size_t datalen); - /// Append copies a single character to the end of the buffer. + /// \brief Append a single character to the end of the buffer. /// /// \param c The character to append. /// \return True if the Buffer was resized. bool Append(const uint8_t c); - /// Insert copies a C-style string into the buffer At index. + /// \brief Insert a C-style string into the buffer at index. /// - /// \param index The index to insert the string At. + /// \note As this is intended for use in text editing, an + /// insert into a buffer after the length will insert + /// spaces before the content. + /// + /// \param index The index to insert the string at. /// \param s The string to insert. /// \return True if the Buffer was resized. bool Insert(const size_t index, const char *s); - /// Insert copies a string into the buffer At index. + /// \brief Insert a string into the buffer at index. /// - /// \param index The index the string should be inserted At. + /// \note As this is intended for use in text editing, an + /// insert into a buffer after the length will insert + /// spaces before the content. + /// + /// \param index The index the string should be inserted at. /// \param s The string to insert. /// \return True if the Buffer was resized. - bool Insert(const size_t index, const std::string s); + bool Insert(const size_t index, const std::string &s); - /// Insert copies a uint8_t buffer into the buffer At index. + /// \brief Insert a uint8_t buffer into the buffer at index. /// - /// \param index The index to insert the buffer At. + /// \note As this is intended for use in text editing, an + /// insert into a buffer after the length will insert + /// spaces before the content. + /// + /// \param index The index to insert the buffer at. /// \param data The buffer to insert. /// \param datalen The size of the data buffer. /// \return True if the Buffer was resized. bool Insert(const size_t index, const uint8_t *data, const size_t datalen); - /// Insert copies a character into the buffer At index. + /// \brief Insert a character into the buffer at index. /// - /// \param index The index to insert the character At. + /// \note As this is intended for use in text editing, an + /// insert into a buffer after the length will insert + /// spaces before the content. + /// + /// \param index The index to insert the character at. /// \param c The character to insert. /// \return True if the Buffer was resized. bool Insert(const size_t index, const uint8_t c); - /// Remove removes `count` bytes from the buffer At `index`. + /// \brief Remove `count` bytes from the buffer at `index`. /// /// \param index The starting index to remove bytes from. /// \param count The number of bytes to remove. /// \return True if the Buffer was resized. bool Remove(const size_t index, const size_t count); - /// Remove removes a single byte from the buffer. + /// \brief Remove removes a single byte from the buffer. /// /// \param index The index pointing to the byte to be removed. /// \return True if the Buffer was resized. @@ -147,7 +164,7 @@ public: /* memory management */ - /// Resize changes the capacity of the buffer to `newCapacity`. + /// \brief Changes the capacity of the buffer to `newCapacity`. /// /// If newCapacity is less than the length of the Buffer, it /// will remove enough bytes from the end to make this happen. @@ -155,27 +172,23 @@ public: /// \param newCapacity The new capacity for the Buffer. void Resize(size_t newCapacity); - /// Trim will resize the Buffer to an appropriate size based on - /// its length. + /// \brief Resize the Buffer capacity based on its length. /// /// \return The new capacity of the Buffer. size_t Trim(); /// DisableAutoTrim prevents the #Buffer from automatically /// trimming memory after a call to #Remove. - void DisableAutoTrim() - { this->autoTrim = false; } + void DisableAutoTrim(); /// EnableAutoTrim enables automatically trimming memory after /// calls to #Remove. - void EnableAutoTrim() - { this->autoTrim = true; } + void EnableAutoTrim(); /// AutoTrimIsEnabled returns true if autotrim is enabled. /// /// \return #Remove will call Trim. - bool AutoTrimIsEnabled() - { return this->autoTrim; } + bool AutoTrimIsEnabled(); /// Clear removes the data stored in the buffer. It will not /// call #Trim; the capacity of the buffer will not be altered. diff --git a/src/bin/phonebook.cc b/src/bin/phonebook.cc index 5b46540..57f5fa9 100644 --- a/src/bin/phonebook.cc +++ b/src/bin/phonebook.cc @@ -186,27 +186,25 @@ main(int argc, char *argv[]) auto args = flags->Args(); args.erase(args.begin()); - auto result = commander.Run(command, args); + delete flags; + switch (result) { case Subcommand::Status::OK: std::cout << "[+] OK\n"; retc = 0; break; case Subcommand::Status::NotEnoughArgs: - delete flags; usage(cerr, 1); break; case Subcommand::Status::Failed: cerr << "[!] '"<< command << "' failed\n"; break; case Subcommand::Status::CommandNotRegistered: - delete flags; cerr << "[!] '" << command << "' not registered.\n"; usage(cerr, 1); break; default: - delete flags; abort(); } diff --git a/src/sl/Arena.cc b/src/sl/Arena.cc index 465c92e..1b060ea 100644 --- a/src/sl/Arena.cc +++ b/src/sl/Arena.cc @@ -20,24 +20,19 @@ /// PERFORMANCE OF THIS SOFTWARE. /// -#include -#include -#include - - -#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) #include #include #include #include - -#define PROT_RW (PROT_WRITE|PROT_READ) -#endif - +#include +#include +#include #include #include +#define PROT_RW (PROT_WRITE|PROT_READ) + namespace scsl { @@ -74,9 +69,6 @@ Arena::SetAlloc(size_t allocSize) this->arenaType = ArenaType::Alloc; this->size = allocSize; this->store = new uint8_t[allocSize]; - if (this->store == nullptr) { - return -1; - } this->Clear(); return 0; @@ -221,16 +213,15 @@ Arena::Destroy() this->arenaType = ArenaType::Uninit; this->size = 0; this->store = nullptr; - return; } std::ostream & operator<<(std::ostream &os, Arena &arena) { - auto cursor = arena.Start(); + auto *cursor = arena.Start(); char cursorString[33] = {0}; snprintf(cursorString, 32, "%#016llx", - (long long unsigned int) cursor); + (long long unsigned int)(cursor)); os << "Arena<"; switch (arena.Type()) { @@ -250,7 +241,7 @@ operator<<(std::ostream &os, Arena &arena) os << "unknown (this is a bug)"; } os << ">@0x"; - os << std::hex << (uintptr_t) &arena; + os << std::hex << static_cast(&arena); os << std::dec; os << ",store<" << arena.Size() << "B>@"; os << std::hex << cursorString; diff --git a/src/sl/Buffer.cc b/src/sl/Buffer.cc index 8775c38..2c9f52d 100644 --- a/src/sl/Buffer.cc +++ b/src/sl/Buffer.cc @@ -83,13 +83,46 @@ Buffer::Buffer(const char *data) } -Buffer::Buffer(const std::string s) +Buffer::Buffer(const std::string& s) : contents(nullptr), length(0), capacity(0), autoTrim(true) { this->Append(s); } +Buffer::~Buffer() +{ + this->Reclaim(); +} + +uint8_t * +Buffer::Contents() const +{ + return this->contents; +} + + +std::string +Buffer::ToString() const +{ + return std::string((const char *)(this->contents)); +} + + +size_t +Buffer::Length() const +{ + return this->length; +} + + +size_t +Buffer::Capacity() const +{ + return this->capacity; +} + + bool Buffer::Append(const char *s) { @@ -100,7 +133,7 @@ Buffer::Append(const char *s) bool -Buffer::Append(const std::string s) +Buffer::Append(const std::string &s) { return this->Append((const uint8_t *) s.c_str(), s.size()); } @@ -156,7 +189,7 @@ Buffer::Insert(const size_t index, const char *s) bool -Buffer::Insert(const size_t index, const std::string s) +Buffer::Insert(const size_t index, const std::string &s) { return this->Insert(index, (const uint8_t *) s.c_str(), s.size()); } @@ -240,6 +273,28 @@ Buffer::Trim() return 0; } + +void +Buffer::DisableAutoTrim() +{ + this->autoTrim = false; +} + + +void +Buffer::EnableAutoTrim() +{ + this->autoTrim = true; +} + + +bool +Buffer::AutoTrimIsEnabled() +{ + return this->autoTrim; +} + + void Buffer::Clear() { @@ -328,7 +383,11 @@ Buffer::shiftRight(size_t offset, size_t delta) resized = true; } - if (this->length == 0) return 0; + if (this->length < offset) { + for (size_t i = this->length; i < offset; i++) { + this->contents[i] = ' '; + } + } memmove(this->contents + (offset + delta), this->contents + offset, diff --git a/src/sl/Commander.cc b/src/sl/Commander.cc index 4f1a1ac..ceb5e10 100644 --- a/src/sl/Commander.cc +++ b/src/sl/Commander.cc @@ -46,7 +46,6 @@ Subcommand::Run(std::vector args) } Commander::Commander() - : cmap() { this->cmap.clear(); } @@ -72,7 +71,7 @@ Commander::Run(std::string command, std::vector args) } auto scmd = this->cmap[command]; - return scmd->Run(args); + return scmd->Run(std::move(args)); } diff --git a/src/sl/Flags.cc b/src/sl/Flags.cc index 15d22c6..fc70569 100644 --- a/src/sl/Flags.cc +++ b/src/sl/Flags.cc @@ -208,11 +208,11 @@ Flags::parseArg(int argc, char **argv, int &index) std::string arg(argv[index]); string::TrimWhitespace(arg); - index++; if (!std::regex_search(arg, isFlag)) { return ParseStatus::EndOfFlags; } + index++; if (this->flags.count(arg) == 0) { if (arg == "-h" || arg == "--help") { Usage(std::cout, 0); diff --git a/test/buffer.cc b/test/buffer.cc index a1a5270..827cb18 100644 --- a/test/buffer.cc +++ b/test/buffer.cc @@ -61,11 +61,6 @@ bufferTest() SCTEST_CHECK_EQ(buffer.Capacity(), 0); buffer.Append("and now for something completely different..."); - buffer.Resize(128); - SCTEST_CHECK_EQ(buffer.Capacity(), 128); - buffer.Trim(); - SCTEST_CHECK_EQ(buffer.Capacity(), 64); - Buffer buffer2("and now for something completely different..."); SCTEST_CHECK_EQ(buffer, buffer2); @@ -76,6 +71,63 @@ bufferTest() } +bool +testBufferTrimming() +{ + const std::string contents = "and now for something completely different..."; + Buffer buffer(contents); + + buffer.Resize(128); + SCTEST_CHECK_EQ(buffer.Capacity(), 128); + + buffer.Trim(); + SCTEST_CHECK_EQ(buffer.Capacity(), 64); + + buffer.DisableAutoTrim(); + buffer.Resize(128); + SCTEST_CHECK_EQ(buffer.Capacity(), 128); + + buffer.Remove(buffer.Length() - 1); + SCTEST_CHECK_EQ(buffer.Capacity(), 128); + + buffer.Append('.'); + SCTEST_CHECK_EQ(buffer.Capacity(), 128); + + buffer.EnableAutoTrim(); + buffer.Remove(buffer.Length() - 1); + SCTEST_CHECK_EQ(buffer.Capacity(), 64); + + return true; +} + + +bool +testInserts() +{ + const std::string contents = "and now for something completely different..."; + const std::string expected1 = " and now for something completely different..."; + const std::string hello = "hello"; + const std::string world = "world"; + const std::string helloWorld = "hello world"; + + Buffer buffer(64); + + // Insert shouldn't resize the buffer. + SCTEST_CHECK_FALSE(buffer.Insert(5, contents)); + SCTEST_CHECK_EQ(buffer.ToString(), expected1); + + buffer.Clear(); + SCTEST_CHECK_EQ(buffer.Length(), 0); + + buffer.Append(hello); + SCTEST_CHECK_EQ(buffer.ToString(), hello); + + buffer.Insert(7, world); + SCTEST_CHECK_EQ(buffer.ToString(), helloWorld); + return true; +} + + int main(int argc, char *argv[]) { @@ -101,6 +153,8 @@ main(int argc, char *argv[]) } suite.AddTest("bufferTest", bufferTest); + suite.AddTest("trimTest", testBufferTrimming); + suite.AddTest("insertTest", testInserts); delete flags; auto result = suite.Run(); diff --git a/test/dictionary.cc b/test/dictionary.cc index fdff67b..0cbab33 100644 --- a/test/dictionary.cc +++ b/test/dictionary.cc @@ -103,6 +103,7 @@ dictionaryTest() SCTEST_CHECK(dict.Lookup(TEST_KVSTR4, TEST_KVSTRLEN4, value)); SCTEST_CHECK(cmpRecord(value, expect)); + arena.Write("pb.dat"); arena.Clear(); return true; }