diff --git a/src/main/native/cpp/Storage.h b/src/main/native/cpp/Storage.h index 096ef8713e..af7318ad9d 100644 --- a/src/main/native/cpp/Storage.h +++ b/src/main/native/cpp/Storage.h @@ -11,9 +11,7 @@ #include #include #include -#include #include -#include #include #include @@ -26,8 +24,13 @@ #include "IStorage.h" +namespace llvm { +class raw_ostream; +} + namespace wpi { class Logger; +class raw_istream; } namespace nt { @@ -113,9 +116,9 @@ class Storage : public IStorage { // Stream-based save/load functions (exposed for testing purposes). These // implement the guts of the filename-based functions. - void SavePersistent(std::ostream& os, bool periodic) const; + void SavePersistent(llvm::raw_ostream& os, bool periodic) const; bool LoadPersistent( - std::istream& is, + wpi::raw_istream& is, std::function warn); // RPC configuration needs to come through here as RPC definitions are diff --git a/src/main/native/cpp/Storage_load.cpp b/src/main/native/cpp/Storage_load.cpp index 2cf1eb35fb..ee24d933a9 100644 --- a/src/main/native/cpp/Storage_load.cpp +++ b/src/main/native/cpp/Storage_load.cpp @@ -10,8 +10,10 @@ #include #include +#include "llvm/SmallString.h" #include "llvm/StringExtras.h" #include "support/Base64.h" +#include "support/raw_istream.h" #include "IDispatcher.h" #include "IEntryNotifier.h" @@ -25,7 +27,7 @@ class LoadPersistentImpl { typedef std::pair> Entry; typedef std::function WarnFunc; - LoadPersistentImpl(std::istream& is, WarnFunc warn) + LoadPersistentImpl(wpi::raw_istream& is, WarnFunc warn) : m_is(is), m_warn(warn) {} bool Load(std::vector* entries); @@ -34,7 +36,7 @@ class LoadPersistentImpl { bool ReadLine(); bool ReadHeader(); NT_Type ReadType(); - bool ReadName(std::string* name); + llvm::StringRef ReadName(llvm::SmallVectorImpl& buf); std::shared_ptr ReadValue(NT_Type type); std::shared_ptr ReadBooleanValue(); std::shared_ptr ReadDoubleValue(); @@ -48,14 +50,13 @@ class LoadPersistentImpl { if (m_warn) m_warn(m_line_num, msg); } - std::istream& m_is; + wpi::raw_istream& m_is; WarnFunc m_warn; llvm::StringRef m_line; - std::string m_line_buf; + llvm::SmallString<128> m_line_buf; std::size_t m_line_num = 0; - std::string m_buf_str; std::vector m_buf_boolean_array; std::vector m_buf_double_array; std::vector m_buf_string_array; @@ -98,29 +99,30 @@ static int fromxdigit(char ch) { return ch - '0'; } -static void UnescapeString(llvm::StringRef source, std::string* dest) { +static llvm::StringRef UnescapeString(llvm::StringRef source, + llvm::SmallVectorImpl& buf) { assert(source.size() >= 2 && source.front() == '"' && source.back() == '"'); - dest->clear(); - dest->reserve(source.size() - 2); + buf.clear(); + buf.reserve(source.size() - 2); for (auto s = source.begin() + 1, end = source.end() - 1; s != end; ++s) { if (*s != '\\') { - dest->push_back(*s); + buf.push_back(*s); continue; } switch (*++s) { case '\\': case '"': - dest->push_back(s[-1]); + buf.push_back(s[-1]); break; case 't': - dest->push_back('\t'); + buf.push_back('\t'); break; case 'n': - dest->push_back('\n'); + buf.push_back('\n'); break; case 'x': { if (!isxdigit(*(s + 1))) { - dest->push_back('x'); // treat it like a unknown escape + buf.push_back('x'); // treat it like a unknown escape break; } int ch = fromxdigit(*++s); @@ -128,22 +130,20 @@ static void UnescapeString(llvm::StringRef source, std::string* dest) { ch <<= 4; ch |= fromxdigit(*++s); } - dest->push_back(static_cast(ch)); + buf.push_back(static_cast(ch)); break; } default: - dest->push_back(s[-1]); + buf.push_back(s[-1]); break; } } + return llvm::StringRef{buf.data(), buf.size()}; } bool LoadPersistentImpl::Load(std::vector* entries) { if (!ReadHeader()) return false; // header - // declare this outside the loop to reduce reallocs - std::string name; - while (ReadLine()) { // type NT_Type type = ReadType(); @@ -153,7 +153,9 @@ bool LoadPersistentImpl::Load(std::vector* entries) { } // name - if (!ReadName(&name)) continue; + llvm::SmallString<128> buf; + llvm::StringRef name = ReadName(buf); + if (name.empty()) continue; // = m_line = m_line.ltrim(" \t"); @@ -167,17 +169,16 @@ bool LoadPersistentImpl::Load(std::vector* entries) { auto value = ReadValue(type); // move to entries - if (!name.empty() && value) - entries->emplace_back(std::move(name), std::move(value)); + if (value) entries->emplace_back(name, std::move(value)); } return true; } bool LoadPersistentImpl::ReadLine() { // ignore blank lines and lines that start with ; or # (comments) - while (std::getline(m_is, m_line_buf)) { + while (!m_is.has_error()) { ++m_line_num; - m_line = llvm::StringRef(m_line_buf).trim(); + m_line = m_is.getline(m_line_buf, INT_MAX).trim(); if (!m_line.empty() && m_line.front() != ';' && m_line.front() != '#') return true; } @@ -217,19 +218,18 @@ NT_Type LoadPersistentImpl::ReadType() { return NT_UNASSIGNED; } -bool LoadPersistentImpl::ReadName(std::string* name) { +llvm::StringRef LoadPersistentImpl::ReadName(llvm::SmallVectorImpl& buf) { llvm::StringRef tok; std::tie(tok, m_line) = ReadStringToken(m_line); if (tok.empty()) { Warn("missing name"); - return false; + return llvm::StringRef{}; } if (tok.back() != '"') { Warn("unterminated name string"); - return false; + return llvm::StringRef{}; } - UnescapeString(tok, name); - return true; + return UnescapeString(tok, buf); } std::shared_ptr LoadPersistentImpl::ReadValue(NT_Type type) { @@ -263,10 +263,9 @@ std::shared_ptr LoadPersistentImpl::ReadBooleanValue() { std::shared_ptr LoadPersistentImpl::ReadDoubleValue() { // need to convert to null-terminated string for strtod() - m_buf_str.clear(); - m_buf_str += m_line; + llvm::SmallString<64> buf; char* end; - double v = std::strtod(m_buf_str.c_str(), &end); + double v = std::strtod(m_line.c_str(buf), &end); if (*end != '\0') { Warn("invalid double value"); return nullptr; @@ -285,13 +284,14 @@ std::shared_ptr LoadPersistentImpl::ReadStringValue() { Warn("unterminated string value"); return nullptr; } - UnescapeString(tok, &m_buf_str); - return Value::MakeString(std::move(m_buf_str)); + llvm::SmallString<128> buf; + return Value::MakeString(UnescapeString(tok, buf)); } std::shared_ptr LoadPersistentImpl::ReadRawValue() { - wpi::Base64Decode(m_line, &m_buf_str); - return Value::MakeRaw(std::move(m_buf_str)); + llvm::SmallString<128> buf; + std::size_t nr; + return Value::MakeRaw(wpi::Base64Decode(m_line, &nr, buf)); } std::shared_ptr LoadPersistentImpl::ReadBooleanArrayValue() { @@ -319,10 +319,9 @@ std::shared_ptr LoadPersistentImpl::ReadDoubleArrayValue() { std::tie(tok, m_line) = m_line.split(','); tok = tok.trim(" \t"); // need to convert to null-terminated string for strtod() - m_buf_str.clear(); - m_buf_str += tok; + llvm::SmallString<64> buf; char* end; - double v = std::strtod(m_buf_str.c_str(), &end); + double v = std::strtod(tok.c_str(buf), &end); if (*end != '\0') { Warn("invalid double value"); return nullptr; @@ -347,8 +346,8 @@ std::shared_ptr LoadPersistentImpl::ReadStringArrayValue() { return nullptr; } - UnescapeString(tok, &m_buf_str); - m_buf_string_array.push_back(std::move(m_buf_str)); + llvm::SmallString<128> buf; + m_buf_string_array.push_back(UnescapeString(tok, buf)); m_line = m_line.ltrim(" \t"); if (m_line.empty()) break; @@ -363,7 +362,7 @@ std::shared_ptr LoadPersistentImpl::ReadStringArrayValue() { } bool Storage::LoadPersistent( - std::istream& is, + wpi::raw_istream& is, std::function warn) { // entries to add std::vector entries; @@ -434,8 +433,9 @@ bool Storage::LoadPersistent( const char* Storage::LoadPersistent( StringRef filename, std::function warn) { - std::ifstream is(filename); - if (!is) return "could not open file"; + std::error_code ec; + wpi::raw_fd_istream is(filename, ec); + if (ec.value() != 0) return "could not open file"; if (!LoadPersistent(is, warn)) return "error reading file"; return nullptr; } diff --git a/src/main/native/cpp/Storage_save.cpp b/src/main/native/cpp/Storage_save.cpp index c9efafd9b3..4463dbd114 100644 --- a/src/main/native/cpp/Storage_save.cpp +++ b/src/main/native/cpp/Storage_save.cpp @@ -10,7 +10,10 @@ #include #include +#include "llvm/Format.h" +#include "llvm/SmallString.h" #include "llvm/StringExtras.h" +#include "llvm/raw_ostream.h" #include "support/Base64.h" #include "Log.h" @@ -23,7 +26,7 @@ class SavePersistentImpl { public: typedef std::pair> Entry; - SavePersistentImpl(std::ostream& os) : m_os(os) {} + SavePersistentImpl(llvm::raw_ostream& os) : m_os(os) {} void Save(llvm::ArrayRef entries); @@ -35,7 +38,7 @@ class SavePersistentImpl { bool WriteType(NT_Type type); void WriteValue(const Value& value); - std::ostream& m_os; + llvm::raw_ostream& m_os; }; } // anonymous namespace @@ -131,15 +134,13 @@ void SavePersistentImpl::WriteValue(const Value& value) { m_os << (value.GetBoolean() ? "true" : "false"); break; case NT_DOUBLE: - m_os << value.GetDouble(); + m_os << llvm::format("%g", value.GetDouble()); break; case NT_STRING: WriteString(value.GetString()); break; case NT_RAW: { - std::string base64_encoded; - wpi::Base64Encode(value.GetRaw(), &base64_encoded); - m_os << base64_encoded; + wpi::Base64Encode(m_os, value.GetRaw()); break; } case NT_BOOLEAN_ARRAY: { @@ -156,7 +157,7 @@ void SavePersistentImpl::WriteValue(const Value& value) { for (auto elem : value.GetDoubleArray()) { if (!first) m_os << ','; first = false; - m_os << elem; + m_os << llvm::format("%g", elem); } break; } @@ -174,17 +175,17 @@ void SavePersistentImpl::WriteValue(const Value& value) { } } -void Storage::SavePersistent(std::ostream& os, bool periodic) const { +void Storage::SavePersistent(llvm::raw_ostream& os, bool periodic) const { std::vector entries; if (!GetPersistentEntries(periodic, &entries)) return; SavePersistentImpl(os).Save(entries); } const char* Storage::SavePersistent(StringRef filename, bool periodic) const { - std::string fn = filename; - std::string tmp = filename; + llvm::SmallString<128> fn = filename; + llvm::SmallString<128> tmp = filename; tmp += ".tmp"; - std::string bak = filename; + llvm::SmallString<128> bak = filename; bak += ".bak"; // Get entries before creating file @@ -194,21 +195,20 @@ const char* Storage::SavePersistent(StringRef filename, bool periodic) const { const char* err = nullptr; // start by writing to temporary file - std::ofstream os(tmp); - if (!os) { + std::error_code ec; + llvm::raw_fd_ostream os(tmp, ec, llvm::sys::fs::F_Text); + if (ec.value() != 0) { err = "could not open file"; goto done; } DEBUG("saving persistent file '" << filename << "'"); SavePersistentImpl(os).Save(entries); - os.flush(); - if (!os) { - os.close(); + os.close(); + if (os.has_error()) { std::remove(tmp.c_str()); err = "error saving file"; goto done; } - os.close(); // Safely move to real file. We ignore any failures related to the backup. std::remove(bak.c_str()); diff --git a/src/test/native/cpp/StorageTest.cpp b/src/test/native/cpp/StorageTest.cpp index de00a2527e..5b5aa7f0c6 100644 --- a/src/test/native/cpp/StorageTest.cpp +++ b/src/test/native/cpp/StorageTest.cpp @@ -8,10 +8,10 @@ #include "StorageTest.h" #include "Storage.h" -#include - #include "gtest/gtest.h" #include "gmock/gmock.h" +#include "llvm/raw_ostream.h" +#include "support/raw_istream.h" #include "MessageMatcher.h" #include "TestPrinters.h" @@ -541,16 +541,18 @@ TEST_P(StorageTestPopulated, GetEntryInfoPrefixTypes) { } TEST_P(StorageTestPersistent, SavePersistentEmpty) { - std::ostringstream oss; + llvm::SmallString<256> buf; + llvm::raw_svector_ostream oss(buf); storage.SavePersistent(oss, false); ASSERT_EQ("[NetworkTables Storage 3.0]\n", oss.str()); } TEST_P(StorageTestPersistent, SavePersistent) { for (auto& i : entries()) i.getValue()->flags = NT_PERSISTENT; - std::ostringstream oss; + llvm::SmallString<256> buf; + llvm::raw_svector_ostream oss(buf); storage.SavePersistent(oss, false); - std::string out = oss.str(); + llvm::StringRef out = oss.str(); //fputs(out.c_str(), stderr); llvm::StringRef line, rem = out; std::tie(line, rem) = rem.split('\n'); @@ -613,16 +615,17 @@ TEST_P(StorageTestEmpty, LoadPersistentBadHeader) { warn.Warn(line, msg); }; - std::istringstream iss(""); - EXPECT_CALL( - warn, - Warn(0, llvm::StringRef("header line mismatch, ignoring rest of file"))); - EXPECT_FALSE(storage.LoadPersistent(iss, warn_func)); - - std::istringstream iss2("[NetworkTables"); + wpi::raw_mem_istream iss(""); EXPECT_CALL( warn, Warn(1, llvm::StringRef("header line mismatch, ignoring rest of file"))); + EXPECT_FALSE(storage.LoadPersistent(iss, warn_func)); + + wpi::raw_mem_istream iss2("[NetworkTables"); + EXPECT_CALL( + warn, + Warn(1, llvm::StringRef("header line mismatch, ignoring rest of file"))); + EXPECT_FALSE(storage.LoadPersistent(iss2, warn_func)); EXPECT_TRUE(entries().empty()); EXPECT_TRUE(idmap().empty()); @@ -634,7 +637,7 @@ TEST_P(StorageTestEmpty, LoadPersistentCommentHeader) { warn.Warn(line, msg); }; - std::istringstream iss( + wpi::raw_mem_istream iss( "\n; comment\n# comment\n[NetworkTables Storage 3.0]\n"); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); EXPECT_TRUE(entries().empty()); @@ -647,7 +650,7 @@ TEST_P(StorageTestEmpty, LoadPersistentEmptyName) { warn.Warn(line, msg); }; - std::istringstream iss( + wpi::raw_mem_istream iss( "[NetworkTables Storage 3.0]\nboolean \"\"=true\n"); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); EXPECT_TRUE(entries().empty()); @@ -671,7 +674,7 @@ TEST_P(StorageTestEmpty, LoadPersistentAssign) { ValueEq(Value::MakeBoolean(true)), NT_NOTIFY_NEW | NT_NOTIFY_LOCAL, UINT_MAX)); - std::istringstream iss( + wpi::raw_mem_istream iss( "[NetworkTables Storage 3.0]\nboolean \"foo\"=true\n"); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); auto entry = GetEntry("foo"); @@ -696,7 +699,7 @@ TEST_P(StorageTestPopulated, LoadPersistentUpdateFlags) { NotifyEntry(1, StringRef("foo2"), ValueEq(Value::MakeDouble(0)), NT_NOTIFY_FLAGS | NT_NOTIFY_LOCAL, UINT_MAX)); - std::istringstream iss( + wpi::raw_mem_istream iss( "[NetworkTables Storage 3.0]\ndouble \"foo2\"=0.0\n"); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); auto entry = GetEntry("foo2"); @@ -725,7 +728,7 @@ TEST_P(StorageTestPopulated, LoadPersistentUpdateValue) { NotifyEntry(1, StringRef("foo2"), ValueEq(Value::MakeDouble(1)), NT_NOTIFY_UPDATE | NT_NOTIFY_LOCAL, UINT_MAX)); - std::istringstream iss( + wpi::raw_mem_istream iss( "[NetworkTables Storage 3.0]\ndouble \"foo2\"=1.0\n"); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); auto entry = GetEntry("foo2"); @@ -761,7 +764,7 @@ TEST_P(StorageTestPopulated, LoadPersistentUpdateValueFlags) { NT_NOTIFY_FLAGS | NT_NOTIFY_UPDATE | NT_NOTIFY_LOCAL, UINT_MAX)); - std::istringstream iss( + wpi::raw_mem_istream iss( "[NetworkTables Storage 3.0]\ndouble \"foo2\"=1.0\n"); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); auto entry = GetEntry("foo2"); @@ -809,7 +812,7 @@ TEST_P(StorageTestEmpty, LoadPersistent) { NotifyEntry(_, _, _, NT_NOTIFY_NEW | NT_NOTIFY_LOCAL, UINT_MAX)) .Times(22); - std::istringstream iss(in); + wpi::raw_mem_istream iss(in); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); ASSERT_EQ(22u, entries().size()); @@ -858,7 +861,7 @@ TEST_P(StorageTestEmpty, LoadPersistentWarn) { warn.Warn(line, msg); }; - std::istringstream iss( + wpi::raw_mem_istream iss( "[NetworkTables Storage 3.0]\nboolean \"foo\"=foo\n"); EXPECT_CALL( warn, Warn(2, llvm::StringRef(