Alter FileReader's memory management
Make it so that the FileReader allows control of the memory before
at the callsite rather than over the whole lifetime of the FileReader
object.
Change-Id: Iee418af8dfa014dcf9718e9cff549ae69e5bc569
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/util/BUILD b/aos/util/BUILD
index 929b376..2f10a70 100644
--- a/aos/util/BUILD
+++ b/aos/util/BUILD
@@ -346,6 +346,7 @@
target_compatible_with = ["@platforms//os:linux"],
deps = [
"//aos/scoped:scoped_fd",
+ "@com_github_google_flatbuffers//:flatbuffers",
"@com_github_google_glog//:glog",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/types:span",
diff --git a/aos/util/file.cc b/aos/util/file.cc
index 6c35627..4e2d1cd 100644
--- a/aos/util/file.cc
+++ b/aos/util/file.cc
@@ -165,12 +165,46 @@
return span;
}
+FileReader::FileReader(std::string_view filename)
+ : file_(open(::std::string(filename).c_str(), O_RDONLY)) {
+ PCHECK(file_.get() != -1) << ": opening " << filename;
+}
+
+absl::Span<char> FileReader::ReadContents(absl::Span<char> buffer) {
+ PCHECK(0 == lseek(file_.get(), 0, SEEK_SET));
+ const ssize_t result = read(file_.get(), buffer.data(), buffer.size());
+ PCHECK(result >= 0);
+ return {buffer.data(), static_cast<size_t>(result)};
+}
+
FileWriter::FileWriter(std::string_view filename, mode_t permissions)
: file_(open(::std::string(filename).c_str(), O_WRONLY | O_CREAT | O_TRUNC,
permissions)) {
PCHECK(file_.get() != -1) << ": opening " << filename;
}
+// absl::SimpleAtoi doesn't interpret a leading 0x as hex, which we need here.
+// Instead, we use the flatbufers API, which unfortunately relies on NUL
+// termination.
+int32_t FileReader::ReadInt32() {
+ // Maximum characters for a 32-bit integer, +1 for the NUL.
+ // Hex is the same size with the leading 0x.
+ std::array<char, 11> buffer;
+ int32_t result;
+ const auto string_span =
+ ReadContents(absl::Span<char>(buffer.data(), buffer.size())
+ .subspan(0, buffer.size() - 1));
+ // Verify we found the newline.
+ CHECK_EQ(buffer[string_span.size() - 1], '\n');
+ // Truncate the newline.
+ buffer[string_span.size() - 1] = '\0';
+ CHECK(flatbuffers::StringToNumber(buffer.data(), &result))
+ << ": Error parsing string to integer: "
+ << std::string_view(string_span.data(), string_span.size());
+
+ return result;
+}
+
FileWriter::WriteResult FileWriter::WriteBytes(
absl::Span<const uint8_t> bytes) {
size_t size_written = 0;
diff --git a/aos/util/file.h b/aos/util/file.h
index 56479ce..ddd5d47 100644
--- a/aos/util/file.h
+++ b/aos/util/file.h
@@ -6,12 +6,14 @@
#include <sys/types.h>
#include <memory>
+#include <optional>
#include <string>
#include <string_view>
#include "absl/strings/numbers.h"
#include "absl/types/span.h"
#include "aos/scoped/scoped_fd.h"
+#include "flatbuffers/util.h"
#include "glog/logging.h"
namespace aos {
@@ -48,43 +50,38 @@
// Wrapper to handle reading the contents of a file into a buffer. Meant for
// situations where the malloc'ing of ReadFileToStringOrDie is inappropriate,
// but where you still want to read a file.
-template <int kBufferSize = 1024>
class FileReader {
public:
- FileReader(std::string_view filename)
- : file_(open(::std::string(filename).c_str(), O_RDONLY)) {
- PCHECK(file_.get() != -1) << ": opening " << filename;
- memset(buffer_, 0, kBufferSize);
- }
+ FileReader(std::string_view filename);
// Reads the entire contents of the file into the internal buffer and returns
// a string_view of it.
// Note: The result may not be null-terminated.
- std::string_view ReadContents() {
- PCHECK(0 == lseek(file_.get(), 0, SEEK_SET));
- const ssize_t result = read(file_.get(), buffer_, sizeof(buffer_));
- PCHECK(result >= 0);
- return {buffer_, static_cast<size_t>(result)};
+ absl::Span<char> ReadContents(absl::Span<char> buffer);
+ // Returns the value of the file as a string, for a fixed-length file.
+ // Returns nullopt if the result is smaller than kSize. Ignores any
+ // bytes beyond kSize.
+ template <int kSize>
+ std::optional<std::array<char, kSize>> ReadString() {
+ std::array<char, kSize> result;
+ const absl::Span<char> used_span =
+ ReadContents(absl::Span<char>(result.data(), result.size()));
+ if (used_span.size() == kSize) {
+ return result;
+ } else {
+ return std::nullopt;
+ }
}
- // Calls ReadContents() and attempts to convert the result into an integer, or
- // dies trying.
- int ReadInt() {
- int result;
- std::string_view contents = ReadContents();
- CHECK(absl::SimpleAtoi(contents, &result))
- << "Failed to parse \"" << contents << "\" as int.";
- return result;
- }
+ // Returns the value of the file as an integer. Crashes if it doesn't fit in a
+ // 32-bit integer. The value may start with 0x for a hex value, otherwise it
+ // must be base 10.
+ int32_t ReadInt32();
private:
aos::ScopedFD file_;
- char buffer_[kBufferSize];
};
// Simple interface to allow opening a file for writing and then writing it
// without any malloc's.
-// TODO(james): It may make sense to add a ReadBytes() interface here that can
-// take a memory buffer to fill, to avoid the templating required by the
-// self-managed buffer of FileReader<>.
class FileWriter {
public:
// The result of an individual call to WriteBytes().
diff --git a/aos/util/file_test.cc b/aos/util/file_test.cc
index 9b0def0..d4382c4 100644
--- a/aos/util/file_test.cc
+++ b/aos/util/file_test.cc
@@ -56,8 +56,20 @@
FLAGS_die_on_malloc = true;
RegisterMallocHook();
aos::ScopedRealtime realtime;
- EXPECT_EQ("123456789\n", reader.ReadContents());
- EXPECT_EQ(123456789, reader.ReadInt());
+ {
+ std::array<char, 20> contents;
+ absl::Span<char> read_result =
+ reader.ReadContents({contents.data(), contents.size()});
+ EXPECT_EQ("123456789\n",
+ std::string_view(read_result.data(), read_result.size()));
+ }
+ {
+ std::optional<std::array<char, 10>> read_result = reader.ReadString<10>();
+ ASSERT_TRUE(read_result.has_value());
+ EXPECT_EQ("123456789\n",
+ std::string_view(read_result->data(), read_result->size()));
+ }
+ EXPECT_EQ(123456789, reader.ReadInt32());
}
// Tests that we can write to a file without malloc'ing.