refactored LogFileAccessor into 2 subclasses for reading and writing
diff --git a/aos/linux_code/logging/binary_log_file.cc b/aos/linux_code/logging/binary_log_file.cc
index 896422c..ff3df68 100644
--- a/aos/linux_code/logging/binary_log_file.cc
+++ b/aos/linux_code/logging/binary_log_file.cc
@@ -32,53 +32,6 @@
MapNextPage();
}
-LogFileMessageHeader *LogFileAccessor::GetWritePosition(size_t message_size) {
- if (position_ + message_size + (kAlignment - (message_size % kAlignment)) +
- sizeof(mutex) > kPageSize) {
- char *const temp = current_;
- MapNextPage();
- if (futex_set_value(static_cast<mutex *>(static_cast<void *>(
- &temp[position_])), 2) == -1) {
- LOG(WARNING,
- "futex_set_value(%p, 2) failed with %d: %s. readers will hang\n",
- &temp[position_], errno, strerror(errno));
- }
- Unmap(temp);
- position_ = 0;
- }
- LogFileMessageHeader *const r = static_cast<LogFileMessageHeader *>(
- static_cast<void *>(¤t_[position_]));
- position_ += message_size;
- AlignPosition();
- return r;
-}
-
-const LogFileMessageHeader *LogFileAccessor::ReadNextMessage(bool wait) {
- LogFileMessageHeader *r;
- do {
- r = static_cast<LogFileMessageHeader *>(
- static_cast<void *>(¤t_[position_]));
- if (wait) {
- if (futex_wait(&r->marker) != 0) continue;
- }
- if (r->marker == 2) {
- Unmap(current_);
- MapNextPage();
- position_ = 0;
- r = static_cast<LogFileMessageHeader *>(static_cast<void *>(current_));
- }
- } while (wait && r->marker == 0);
- if (r->marker == 0) {
- return NULL;
- }
- position_ += sizeof(LogFileMessageHeader) + r->name_size + r->message_size;
- AlignPosition();
- if (position_ >= kPageSize) {
- LOG(FATAL, "corrupt log file running over page size\n");
- }
- return r;
-}
-
void LogFileAccessor::Sync() const {
msync(current_, kPageSize, MS_ASYNC | MS_INVALIDATE);
}
@@ -101,10 +54,8 @@
void LogFileAccessor::MapNextPage() {
if (writable_) {
if (ftruncate(fd_, offset_ + kPageSize) == -1) {
- fprintf(stderr, "ftruncate(%d, %zd) failed with %d: %s. aborting\n",
- fd_, kPageSize, errno, strerror(errno));
- printf("see stderr\n");
- abort();
+ LOG(FATAL, "ftruncate(%d, %zd) failed with %d: %s. aborting\n", fd_,
+ kPageSize, errno, strerror(errno));
}
}
current_ = static_cast<char *>(
@@ -123,10 +74,6 @@
current_, kPageSize, errno, strerror(errno));
}
offset_ += kPageSize;
-
- if (!writable_) {
- CheckCurrentPageReadable();
- }
}
void LogFileAccessor::Unmap(void *location) {
@@ -135,6 +82,35 @@
kPageSize, errno, strerror(errno));
}
is_last_page_ = 0;
+ position_ = 0;
+}
+
+const LogFileMessageHeader *LogFileReader::ReadNextMessage(bool wait) {
+ LogFileMessageHeader *r;
+ do {
+ r = static_cast<LogFileMessageHeader *>(
+ static_cast<void *>(¤t()[position()]));
+ if (wait) {
+ if (futex_wait(&r->marker) != 0) continue;
+ }
+ if (r->marker == 2) {
+ Unmap(current());
+ MapNextPage();
+ CheckCurrentPageReadable();
+ r = static_cast<LogFileMessageHeader *>(static_cast<void *>(current()));
+ }
+ } while (wait && r->marker == 0);
+ if (r->marker == 0) {
+ return NULL;
+ }
+ IncrementPosition(sizeof(LogFileMessageHeader) + r->name_size +
+ r->message_size);
+ if (position() >= kPageSize) {
+ // It's a lot better to blow up here rather than getting SIGBUS errors the
+ // next time we try to read...
+ LOG(FATAL, "corrupt log file running over page size\n");
+ }
+ return r;
}
// This mess is because the only not completely hackish way to do this is to set
@@ -153,7 +129,7 @@
}
} // namespace
-void LogFileAccessor::CheckCurrentPageReadable() {
+void LogFileReader::CheckCurrentPageReadable() {
if (sigsetjmp(jump_context, 1) == 0) {
struct sigaction action;
action.sa_sigaction = CheckCurrentPageReadableHandler;
@@ -169,7 +145,7 @@
SIGSEGV, &action, &previous_segv, errno, strerror(errno));
}
- char __attribute__((unused)) c = current_[0];
+ char __attribute__((unused)) c = current()[0];
if (sigaction(SIGBUS, &previous_bus, NULL) == -1) {
LOG(FATAL, "sigaction(SIGBUS(=%d), %p, NULL) failed with %d: %s\n",
@@ -180,16 +156,35 @@
SIGSEGV, &previous_segv, errno, strerror(errno));
}
} else {
- if (fault_address == current_) {
+ if (fault_address == current()) {
LOG(FATAL, "could not read 1 byte at offset 0x%jx into log file\n",
- static_cast<uintmax_t>(offset_));
+ static_cast<uintmax_t>(offset()));
} else {
LOG(FATAL, "faulted at %p, not %p like we were (maybe) supposed to\n",
- fault_address, current_);
+ fault_address, current());
}
}
}
+LogFileMessageHeader *LogFileWriter::GetWritePosition(size_t message_size) {
+ if (position() + message_size + (kAlignment - (message_size % kAlignment)) +
+ sizeof(mutex) > kPageSize) {
+ char *const temp = current();
+ MapNextPage();
+ if (futex_set_value(static_cast<mutex *>(static_cast<void *>(
+ &temp[position()])), 2) == -1) {
+ LOG(WARNING,
+ "futex_set_value(%p, 2) failed with %d: %s. readers will hang\n",
+ &temp[position()], errno, strerror(errno));
+ }
+ Unmap(temp);
+ }
+ LogFileMessageHeader *const r = static_cast<LogFileMessageHeader *>(
+ static_cast<void *>(¤t()[position()]));
+ IncrementPosition(message_size);
+ return r;
+}
+
} // namespace linux_code
} // namespace logging
} // namespace aos
diff --git a/aos/linux_code/logging/binary_log_file.h b/aos/linux_code/logging/binary_log_file.h
index f25482c..54e64d8 100644
--- a/aos/linux_code/logging/binary_log_file.h
+++ b/aos/linux_code/logging/binary_log_file.h
@@ -86,17 +86,12 @@
public:
LogFileAccessor(int fd, bool writable);
- // message_size should be the total number of bytes needed for the message.
- LogFileMessageHeader *GetWritePosition(size_t message_size);
- // May return NULL iff wait is false.
- const LogFileMessageHeader *ReadNextMessage(bool wait);
-
// Asynchronously syncs all open mappings.
void Sync() const;
bool IsLastPage();
- private:
+ protected:
// The size of the chunks that get mmaped/munmapped together. Large enough so
// that not too much space is wasted and it's hopefully bigger than and a
// multiple of the system page size but small enough so that really large
@@ -106,6 +101,24 @@
static const size_t kAlignment = MESSAGE_ALIGNMENT;
#undef MESSAGE_ALIGNMENT
+ char *current() const { return current_; }
+ size_t position() const { return position_; }
+ off_t offset() const { return offset_; }
+
+ void IncrementPosition(size_t size) {
+ position_ += size;
+ AlignPosition();
+ }
+
+ void MapNextPage();
+ void Unmap(void *location);
+
+ // Advances position to the next (aligned) location.
+ void AlignPosition() {
+ position_ += kAlignment - (position_ % kAlignment);
+ }
+
+ private:
const int fd_;
const bool writable_;
@@ -116,17 +129,27 @@
// 0 = unknown, 1 = no, 2 = yes
int is_last_page_ = 0;
+};
- void MapNextPage();
- void Unmap(void *location);
+class LogFileReader : public LogFileAccessor {
+ public:
+ LogFileReader(int fd) : LogFileAccessor(fd, false) {}
+
+ // May return NULL iff wait is false.
+ const LogFileMessageHeader *ReadNextMessage(bool wait);
+
+ private:
// Tries reading from the current page to see if it fails because the file
// isn't big enough.
void CheckCurrentPageReadable();
+};
- // Advances position to the next (aligned) location.
- void AlignPosition() {
- position_ += kAlignment - (position_ % kAlignment);
- }
+class LogFileWriter : public LogFileAccessor {
+ public:
+ LogFileWriter(int fd) : LogFileAccessor(fd, true) {}
+
+ // message_size should be the total number of bytes needed for the message.
+ LogFileMessageHeader *GetWritePosition(size_t message_size);
};
} // namespace linux_code
diff --git a/aos/linux_code/logging/binary_log_writer.cc b/aos/linux_code/logging/binary_log_writer.cc
index dfddef8..37a069b 100644
--- a/aos/linux_code/logging/binary_log_writer.cc
+++ b/aos/linux_code/logging/binary_log_writer.cc
@@ -25,7 +25,7 @@
namespace linux_code {
namespace {
-void CheckTypeWritten(uint32_t type_id, LogFileAccessor &writer) {
+void CheckTypeWritten(uint32_t type_id, LogFileWriter &writer) {
static ::std::unordered_set<uint32_t> written_type_ids;
if (written_type_ids.count(type_id) > 0) return;
if (MessageType::IsPrimitive(type_id)) return;
@@ -139,7 +139,7 @@
" exiting\n", tmp, errno, strerror(errno));
return EXIT_FAILURE;
}
- LogFileAccessor writer(fd, true);
+ LogFileWriter writer(fd);
while (true) {
const LogMessage *const msg = ReadNext();
@@ -156,6 +156,7 @@
output_length +=
sizeof(msg->matrix.type) + sizeof(uint32_t) + sizeof(uint16_t) +
sizeof(uint16_t) + msg->matrix.string_length;
+ CHECK(MessageType::IsPrimitive(msg->matrix.type));
}
LogFileMessageHeader *const output = writer.GetWritePosition(output_length);
char *output_strings = reinterpret_cast<char *>(output) + sizeof(*output);
diff --git a/aos/linux_code/logging/log_displayer.cc b/aos/linux_code/logging/log_displayer.cc
index dd78b91..0bb3508 100644
--- a/aos/linux_code/logging/log_displayer.cc
+++ b/aos/linux_code/logging/log_displayer.cc
@@ -151,7 +151,7 @@
filename, strerror(errno));
exit(EXIT_FAILURE);
}
- ::aos::logging::linux_code::LogFileAccessor accessor(fd, false);
+ ::aos::logging::linux_code::LogFileReader reader(fd);
if (skip_to_end) {
fputs("skipping old logs...\n", stderr);
@@ -159,7 +159,7 @@
const LogFileMessageHeader *msg;
do {
- msg = accessor.ReadNextMessage(follow);
+ msg = reader.ReadNextMessage(follow);
if (msg == NULL) {
fputs("reached end of file\n", stderr);
return 0;
@@ -174,7 +174,7 @@
}
if (skip_to_end) {
- if (accessor.IsLastPage()) {
+ if (reader.IsLastPage()) {
fputs("done skipping old logs\n", stderr);
skip_to_end = false;
} else {