From 043df533b7bec18bb4f5e207b0a0b89adafdafbc Mon Sep 17 00:00:00 2001 From: Gary Talent Date: Fri, 10 May 2024 01:29:13 -0500 Subject: [PATCH] [ox] Cleanup string len handling Remove UTF-8 parsing. It is a rare enough need that it should have a specialized call when needed. Better to have a more optimal length fetch for typical case. --- deps/ox/src/ox/clargs/clargs.cpp | 2 +- deps/ox/src/ox/fs/filesystem/directory.hpp | 10 ++-- deps/ox/src/ox/fs/filesystem/pathiterator.cpp | 43 ++++++-------- deps/ox/src/ox/fs/filesystem/pathiterator.hpp | 14 +---- deps/ox/src/ox/fs/test/tests.cpp | 37 ++++++------ deps/ox/src/ox/mc/read.hpp | 22 ++++++-- deps/ox/src/ox/mc/test/tests.cpp | 3 +- deps/ox/src/ox/mc/write.hpp | 2 +- deps/ox/src/ox/oc/read.hpp | 14 ++++- deps/ox/src/ox/std/CMakeLists.txt | 2 +- deps/ox/src/ox/std/istring.hpp | 56 ++++--------------- deps/ox/src/ox/std/memops.hpp | 2 +- deps/ox/src/ox/std/string.hpp | 20 +------ deps/ox/src/ox/std/test/tests.cpp | 1 + deps/ox/src/ox/std/uuid.hpp | 5 +- 15 files changed, 97 insertions(+), 136 deletions(-) diff --git a/deps/ox/src/ox/clargs/clargs.cpp b/deps/ox/src/ox/clargs/clargs.cpp index b2bee892..90009b2f 100644 --- a/deps/ox/src/ox/clargs/clargs.cpp +++ b/deps/ox/src/ox/clargs/clargs.cpp @@ -44,7 +44,7 @@ bool ClArgs::getBool(ox::CRStringView arg, bool defaultValue) const noexcept { String ClArgs::getString(ox::CRStringView arg, const char *defaultValue) const noexcept { auto [value, err] = m_strings.at(arg); - return !err ? ox::String(std::move(*value)) : ox::String(defaultValue); + return !err ? ox::String(*value) : ox::String(defaultValue); } int ClArgs::getInt(ox::CRStringView arg, int defaultValue) const noexcept { diff --git a/deps/ox/src/ox/fs/filesystem/directory.hpp b/deps/ox/src/ox/fs/filesystem/directory.hpp index 4f9573ed..5548fdd9 100644 --- a/deps/ox/src/ox/fs/filesystem/directory.hpp +++ b/deps/ox/src/ox/fs/filesystem/directory.hpp @@ -159,7 +159,7 @@ Error Directory::mkdir(PathIterator path, bool parents, Fi // determine if already exists auto name = nameBuff; - oxReturnError(path.get(name)); + oxReturnError(path.get(*name)); auto childInode = find(PathIterator(*name)); if (!childInode.ok()) { // if this is not the last item in the path and parents is disabled, @@ -203,7 +203,7 @@ Error Directory::write(PathIterator path, uint64_t inode64 auto name = nameBuff; if (path.next().hasNext()) { // not yet at target directory, recurse to next one - oxReturnError(path.get(name)); + oxReturnError(path.get(*name)); oxTracef("ox.fs.Directory.write", "Attempting to write to next sub-Directory: {} of {}", *name, path.fullPath()); oxRequire(nextChild, findEntry(*name)); @@ -222,7 +222,7 @@ Error Directory::write(PathIterator path, uint64_t inode64 // insert the new entry on this directory // get the name - oxReturnError(path.next(name)); + oxReturnError(path.next(*name)); // find existing version of directory oxTracef("ox.fs.Directory.write", "Searching for directory inode {}", m_inodeId); @@ -263,7 +263,7 @@ Error Directory::remove(PathIterator path, FileName *nameB nameBuff = new (ox_alloca(sizeof(FileName))) FileName; } auto &name = *nameBuff; - oxReturnError(path.get(&name)); + oxReturnError(path.get(name)); oxTrace("ox.fs.Directory.remove", name); auto buff = m_fs.read(m_inodeId).template to(); @@ -343,7 +343,7 @@ Result Directory::find(Path // determine if already exists auto name = nameBuff; - oxReturnError(path.get(name)); + oxReturnError(path.get(*name)); oxRequire(v, findEntry(*name)); // recurse if not at end of path diff --git a/deps/ox/src/ox/fs/filesystem/pathiterator.cpp b/deps/ox/src/ox/fs/filesystem/pathiterator.cpp index 1d330b0f..716d481e 100644 --- a/deps/ox/src/ox/fs/filesystem/pathiterator.cpp +++ b/deps/ox/src/ox/fs/filesystem/pathiterator.cpp @@ -61,8 +61,9 @@ Error PathIterator::fileName(char *out, std::size_t outSize) { } // Gets the get item in the path -Error PathIterator::get(char *pathOut, std::size_t pathOutSize) { +Error PathIterator::get(IString &fileName) { std::size_t size = 0; + std::ignore = fileName.resize(MaxFileNameLength); if (m_iterator >= m_maxSize) { oxTracef("ox.fs.PathIterator.get", "m_iterator ({}) >= m_maxSize ({})", m_iterator, m_maxSize); return OxError(1); @@ -84,22 +85,25 @@ Error PathIterator::get(char *pathOut, std::size_t pathOutSize) { const auto end = static_cast(substr - m_path); size = end - start; // cannot fit the output in the output parameter - if (size >= pathOutSize || size == 0) { + if (size >= MaxFileNameLength || size == 0) { return OxError(1); } - ox::memcpy(pathOut, &m_path[start], size); + ox::memcpy(fileName.data(), &m_path[start], size); // truncate trailing / - if (size && pathOut[size - 1] == '/') { + if (size && fileName[size - 1] == '/') { size--; } - pathOut[size] = 0; // end with null terminator - return OxError(0); + oxReturnError(fileName.resize(size)); + return {}; } -// Gets the get item in the path -Error PathIterator::next(char *pathOut, std::size_t pathOutSize) { +/** + * @return 0 if no error + */ +Error PathIterator::next(IString &fileName) { std::size_t size = 0; auto retval = OxError(1); + std::ignore = fileName.resize(MaxFileNameLength); if (m_iterator < m_maxSize && ox::strlen(&m_path[m_iterator])) { retval = OxError(0); if (m_path[m_iterator] == '/') { @@ -115,34 +119,21 @@ Error PathIterator::next(char *pathOut, std::size_t pathOutSize) { const auto end = static_cast(substr - m_path); size = end - start; // cannot fit the output in the output parameter - if (size >= pathOutSize) { + if (size >= MaxFileNameLength) { return OxError(1); } - ox::memcpy(pathOut, &m_path[start], size); + ox::memcpy(fileName.data(), &m_path[start], size); } // truncate trailing / - if (size && pathOut[size - 1] == '/') { + if (size && fileName[size - 1] == '/') { size--; } - pathOut[size] = 0; // end with null terminator + fileName[size] = 0; // end with null terminator + oxReturnError(fileName.resize(size)); m_iterator += size; return retval; } -/** - * @return 0 if no error - */ -Error PathIterator::get(IString *fileName) { - return get(fileName->data(), fileName->cap()); -} - -/** - * @return 0 if no error - */ -Error PathIterator::next(IString *fileName) { - return next(fileName->data(), fileName->cap()); -} - Result PathIterator::nextSize() const { std::size_t size = 0; auto retval = OxError(1); diff --git a/deps/ox/src/ox/fs/filesystem/pathiterator.hpp b/deps/ox/src/ox/fs/filesystem/pathiterator.hpp index 127198a9..b3cbeda2 100644 --- a/deps/ox/src/ox/fs/filesystem/pathiterator.hpp +++ b/deps/ox/src/ox/fs/filesystem/pathiterator.hpp @@ -41,22 +41,12 @@ class PathIterator { /** * @return 0 if no error */ - Error next(char *pathOut, std::size_t pathOutSize); + Error next(FileName &fileName); /** * @return 0 if no error */ - Error get(char *pathOut, std::size_t pathOutSize); - - /** - * @return 0 if no error - */ - Error next(FileName *fileName); - - /** - * @return 0 if no error - */ - Error get(FileName *fileName); + Error get(FileName &fileName); /** * @return 0 if no error diff --git a/deps/ox/src/ox/fs/test/tests.cpp b/deps/ox/src/ox/fs/test/tests.cpp index b9bdc2d5..25e5cb2a 100644 --- a/deps/ox/src/ox/fs/test/tests.cpp +++ b/deps/ox/src/ox/fs/test/tests.cpp @@ -60,10 +60,10 @@ const std::map> tests = [](ox::StringView) { auto const path = ox::String("/usr/share/charset.gbag"); ox::PathIterator it(path.c_str(), path.len()); - auto buff = static_cast(ox_alloca(path.len() + 1)); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "usr") == 0, "PathIterator shows wrong next"); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "share") == 0, "PathIterator shows wrong next"); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "charset.gbag") == 0, "PathIterator shows wrong next"); + ox::FileName buff; + oxAssert(it.next(buff) == 0 && buff == "usr", "PathIterator shows wrong next"); + oxAssert(it.next(buff) == 0 && buff == "share", "PathIterator shows wrong next"); + oxAssert(it.next(buff) == 0 && buff == "charset.gbag", "PathIterator shows wrong next"); return OxError(0); } }, @@ -72,9 +72,9 @@ const std::map> tests = [](ox::StringView) { auto const path = ox::String("/usr/share/"); ox::PathIterator it(path.c_str(), path.len()); - auto buff = static_cast(ox_alloca(path.len() + 1)); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "usr") == 0, "PathIterator shows wrong next"); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "share") == 0, "PathIterator shows wrong next"); + ox::FileName buff; + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "usr") == 0, "PathIterator shows wrong next"); + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "share") == 0, "PathIterator shows wrong next"); return OxError(0); } }, @@ -83,8 +83,8 @@ const std::map> tests = [](ox::StringView) { auto const path = ox::String("/"); ox::PathIterator it(path.c_str(), path.len()); - auto buff = static_cast(ox_alloca(path.len() + 1)); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "\0") == 0, "PathIterator shows wrong next"); + ox::FileName buff; + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "\0") == 0, "PathIterator shows wrong next"); return OxError(0); } }, @@ -93,10 +93,10 @@ const std::map> tests = [](ox::StringView) { auto const path = ox::String("usr/share/charset.gbag"); ox::PathIterator it(path.c_str(), path.len()); - auto buff = static_cast(ox_alloca(path.len() + 1)); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "usr") == 0, "PathIterator shows wrong next"); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "share") == 0, "PathIterator shows wrong next"); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "charset.gbag") == 0, "PathIterator shows wrong next"); + ox::FileName buff; + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "usr") == 0, "PathIterator shows wrong next"); + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "share") == 0, "PathIterator shows wrong next"); + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "charset.gbag") == 0, "PathIterator shows wrong next"); return OxError(0); } }, @@ -105,9 +105,9 @@ const std::map> tests = [](ox::StringView) { auto const path = ox::String("usr/share/"); ox::PathIterator it(path.c_str(), path.len()); - auto buff = static_cast(ox_alloca(path.len() + 1)); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "usr") == 0, "PathIterator shows wrong next"); - oxAssert(it.next(buff, path.len()) == 0 && ox::strcmp(buff, "share") == 0, "PathIterator shows wrong next"); + ox::FileName buff; + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "usr") == 0, "PathIterator shows wrong next"); + oxAssert(it.next(buff) == 0 && ox::strcmp(buff, "share") == 0, "PathIterator shows wrong next"); return OxError(0); } }, @@ -237,8 +237,9 @@ const std::map> tests = }; int main(int argc, const char **args) { - if (argc < 3) { - oxError("Must specify test to run and test argument"); + if (argc < 2) { + oxError("Must specify test to run"); + return -1; } ox::StringView const testName = args[1]; ox::StringView const testArg = args[2] ? args[2] : nullptr; diff --git a/deps/ox/src/ox/mc/read.hpp b/deps/ox/src/ox/mc/read.hpp index 3cebd2a9..cbb472af 100644 --- a/deps/ox/src/ox/mc/read.hpp +++ b/deps/ox/src/ox/mc/read.hpp @@ -322,9 +322,6 @@ constexpr Error MetalClawReaderTemplate::field(const char*, BasicString< *val = BasicString(cap); auto data = val->data(); // read the string - if (static_cast(cap) < size) { - return OxError(McOutputBuffEnded); - } oxReturnError(m_reader.read(data, size)); } else { *val = ""; @@ -336,8 +333,23 @@ constexpr Error MetalClawReaderTemplate::field(const char*, BasicString< template template -constexpr Error MetalClawReaderTemplate::field(const char *name, IString *val) noexcept { - return fieldCString(name, val->data(), val->cap()); +constexpr Error MetalClawReaderTemplate::field(const char*, IString *val) noexcept { + if (!m_unionIdx.has_value() || static_cast(*m_unionIdx) == m_field) { + if (m_fieldPresence.get(static_cast(m_field))) { + // read the length + std::size_t bytesRead = 0; + oxRequire(size, mc::decodeInteger(m_reader, &bytesRead)); + *val = IString(); + oxReturnError(val->resize(size)); + auto const data = val->data(); + // read the string + oxReturnError(m_reader.read(data, size)); + } else { + *val = ""; + } + } + ++m_field; + return {}; } template diff --git a/deps/ox/src/ox/mc/test/tests.cpp b/deps/ox/src/ox/mc/test/tests.cpp index 456a6bd1..1ff87d10 100644 --- a/deps/ox/src/ox/mc/test/tests.cpp +++ b/deps/ox/src/ox/mc/test/tests.cpp @@ -157,7 +157,8 @@ std::map tests = { oxAssert(testIn.Int8 == testOut.Int8, "Int8 value mismatch"); oxAssert(testIn.Union.Int == testOut.Union.Int, "Union.Int value mismatch"); oxAssert(testIn.String == testOut.String, "String value mismatch"); - oxAssert(testIn.IString == testOut.IString, "IString value mismatch"); + oxDebugf("{}", testOut.IString.len()); + oxExpect(testIn.IString, testOut.IString); oxAssert(testIn.List[0] == testOut.List[0], "List[0] value mismatch"); oxAssert(testIn.List[1] == testOut.List[1], "List[1] value mismatch"); oxAssert(testIn.List[2] == testOut.List[2], "List[2] value mismatch"); diff --git a/deps/ox/src/ox/mc/write.hpp b/deps/ox/src/ox/mc/write.hpp index fb1ed706..49de6764 100644 --- a/deps/ox/src/ox/mc/write.hpp +++ b/deps/ox/src/ox/mc/write.hpp @@ -207,7 +207,7 @@ constexpr Error MetalClawWriter::field(const char*, const BasicString template constexpr Error MetalClawWriter::field(const char *name, const IString *val) noexcept { - return fieldCString(name, val->data(), val->cap()); + return fieldCString(name, val->data(), val->len()); } template diff --git a/deps/ox/src/ox/oc/read.hpp b/deps/ox/src/ox/oc/read.hpp index eebf50a4..b254d49f 100644 --- a/deps/ox/src/ox/oc/read.hpp +++ b/deps/ox/src/ox/oc/read.hpp @@ -207,7 +207,19 @@ Error OrganicClawReader::field(const char *key, BasicString *val) noexcept { template Error OrganicClawReader::field(const char *key, IString *val) noexcept { - return fieldCString(key, val->data(), val->cap()); + auto err = OxError(0); + if (targetValid()) { + const auto &jv = value(key); + if (jv.empty()) { + *val = IString{}; + } else if (jv.isString()) { + *val = jv.asString().c_str(); + } else { + err = OxError(1, "Type mismatch"); + } + } + ++m_fieldIt; + return err; } // array handler diff --git a/deps/ox/src/ox/std/CMakeLists.txt b/deps/ox/src/ox/std/CMakeLists.txt index ebc10c0f..0aface63 100644 --- a/deps/ox/src/ox/std/CMakeLists.txt +++ b/deps/ox/src/ox/std/CMakeLists.txt @@ -45,7 +45,7 @@ add_library( if(NOT MSVC) target_compile_options(OxStd PRIVATE -Wsign-conversion) target_compile_options(OxStd PRIVATE -Wconversion) - if(${OX_OS_LINUX}) + if(${OX_OS_LINUX} AND NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang") target_compile_options(OxStd PUBLIC -export-dynamic) #target_link_options(OxStd PUBLIC -W1,-E) elseif(${OX_OS_FREEBSD}) diff --git a/deps/ox/src/ox/std/istring.hpp b/deps/ox/src/ox/std/istring.hpp index 30ee64f5..8c1d996c 100644 --- a/deps/ox/src/ox/std/istring.hpp +++ b/deps/ox/src/ox/std/istring.hpp @@ -85,7 +85,9 @@ class IString { * Returns the capacity of bytes for this string. */ [[nodiscard]] - constexpr std::size_t cap() const noexcept; + constexpr static std::size_t cap() noexcept { + return StrCap; + } }; template @@ -113,7 +115,7 @@ constexpr IString &IString::operator=(Integer_c auto i) noexcept { template constexpr IString &IString::operator=(ox::CRStringView str) noexcept { - std::size_t strLen = str.bytes() + 1; + std::size_t strLen = str.len(); if (cap() < strLen) { strLen = cap(); } @@ -126,14 +128,7 @@ constexpr IString &IString::operator=(ox::CRStringView str) noexcept template constexpr IString &IString::operator=(const char *str) noexcept { - std::size_t strLen = ox::strlen(str) + 1; - if (cap() < strLen) { - strLen = cap(); - } - m_size = strLen; - ox::listcpy(m_buff.data(), str, strLen); - // make sure last element is a null terminator - m_buff[cap()] = 0; + operator=(ox::StringView{str}); return *this; } @@ -170,30 +165,21 @@ constexpr char &IString::operator[](std::size_t i) noexcept { template constexpr Error IString::append(const char *str, std::size_t strLen) noexcept { Error err{}; - auto currentLen = len(); - if (cap() < currentLen + strLen + 1) { + auto const currentLen = len(); + if (cap() < currentLen + strLen) { strLen = cap() - currentLen; err = OxError(1, "Insufficient space for full string"); } ox::strncpy(m_buff.data() + currentLen, str, strLen); // make sure last element is a null terminator m_buff[currentLen + strLen] = 0; + m_size += strLen; return err; } template constexpr Error IString::append(ox::StringView str) noexcept { - auto strLen = str.len(); - Error err{}; - auto currentLen = len(); - if (cap() < currentLen + strLen + 1) { - strLen = cap() - currentLen; - err = OxError(1, "Insufficient space for full string"); - } - ox::strncpy(m_buff.data() + currentLen, str, strLen); - // make sure last element is a null terminator - m_buff[currentLen + strLen] = 0; - return err; + return append(str.data(), str.len()); } template @@ -214,27 +200,12 @@ constexpr const char *IString::c_str() const noexcept { template constexpr std::size_t IString::len() const noexcept { - std::size_t length = 0; - for (std::size_t i = 0; i < StrCap; i++) { - auto const b = static_cast(m_buff[i]); - if (b) { - const auto asciiChar = (b & 128) == 0; - const auto utf8Char = (b & (256 << 6)) == (256 << 6); - if (asciiChar || utf8Char) { - length++; - } - } else { - break; - } - } - return length; + return m_size; } template constexpr std::size_t IString::bytes() const noexcept { - std::size_t i = 0; - for (i = 0; i < StrCap && m_buff[i]; i++); - return i + 1; // add one for null terminator + return m_size + 1; } template @@ -249,11 +220,6 @@ constexpr ox::Error IString::resize(size_t sz) noexcept { return {}; } -template -constexpr std::size_t IString::cap() const noexcept { - return StrCap; -} - template struct MaybeView> { using type = ox::StringView; diff --git a/deps/ox/src/ox/std/memops.hpp b/deps/ox/src/ox/std/memops.hpp index 3c6e3ba9..202f3a67 100644 --- a/deps/ox/src/ox/std/memops.hpp +++ b/deps/ox/src/ox/std/memops.hpp @@ -33,7 +33,7 @@ template constexpr T1 *listcpy(T1 *dest, T2 *src, std::size_t maxLen) noexcept { using T1Type = typename ox::remove_reference::type; std::size_t i = 0; - while (i < maxLen && src[i]) { + while (i < maxLen) { dest[i] = static_cast(src[i]); ++i; } diff --git a/deps/ox/src/ox/std/string.hpp b/deps/ox/src/ox/std/string.hpp index d5406bba..05d11224 100644 --- a/deps/ox/src/ox/std/string.hpp +++ b/deps/ox/src/ox/std/string.hpp @@ -515,7 +515,7 @@ constexpr char &BasicString::operator[](std::size_t i) noexce template constexpr BasicString BasicString::substr(std::size_t pos) const noexcept { - return BasicString(m_buff.data() + pos, m_buff.size() - pos); + return BasicString(m_buff.data() + pos, m_buff.size() - pos - 1); } template @@ -531,26 +531,12 @@ constexpr BasicString BasicString::substr( template constexpr std::size_t BasicString::bytes() const noexcept { - std::size_t i; - for (i = 0; i < m_buff.size() && m_buff[i]; ++i); - return i + 1; // add one for null terminator + return m_buff.size(); } template constexpr std::size_t BasicString::len() const noexcept { - std::size_t length = 0; - for (const auto c : m_buff) { - const auto b = static_cast(c); - if (b) { - // normal ASCII character or start of UTF-8 character - if ((b & 128) == 0 || (b & (256 << 6)) == (256 << 6)) { - ++length; - } - } else { - break; - } - } - return length; + return m_buff.size() - 1; } template diff --git a/deps/ox/src/ox/std/test/tests.cpp b/deps/ox/src/ox/std/test/tests.cpp index 2a72f17a..83454091 100644 --- a/deps/ox/src/ox/std/test/tests.cpp +++ b/deps/ox/src/ox/std/test/tests.cpp @@ -109,6 +109,7 @@ static std::map tests = { oxAssert(ox::StringView("Write") != ox::String(""), "String / StringView comparison broken"); oxAssert(ox::String("Write") != ox::StringView(""), "String / StringView comparison broken"); oxAssert(ox::String("Write") == ox::StringView("Write"), "String / StringView comparison broken"); + oxExpect(ox::String("asdf").substr(1, 3), "sd"); oxAssert( ox::String(ox::StringView("Write")) == ox::StringView("Write"), "String / StringView comparison broken"); diff --git a/deps/ox/src/ox/std/uuid.hpp b/deps/ox/src/ox/std/uuid.hpp index 19e535f7..d2d783b4 100644 --- a/deps/ox/src/ox/std/uuid.hpp +++ b/deps/ox/src/ox/std/uuid.hpp @@ -176,9 +176,10 @@ class UUID { [[nodiscard]] constexpr UUIDStr toString() const noexcept { UUIDStr out; - ox::CharBuffWriter bw(out.data(), out.cap()); + std::ignore = out.resize(UUIDStr::cap()); + ox::CharBuffWriter bw(out.data(), UUIDStr::cap()); std::ignore = toString(bw); - out[out.cap()] = 0; + out[UUIDStr::cap()] = 0; return out; } };