SingleLogSource handles rotation of log files

chrome/browser/chromeos/system_logs/single_log_source.cc

Index: chrome/browser/chromeos/system_logs/single_log_source.cc
diff --git a/chrome/browser/chromeos/system_logs/single_log_source.cc b/chrome/browser/chromeos/system_logs/single_log_source.cc
index 1d09e8b398b36439db90502e783aa66c0a13c16f..99abc58501a2b9bc5dfaff5ecabbbbebf8eef125 100644
--- a/chrome/browser/chromeos/system_logs/single_log_source.cc
+++ b/chrome/browser/chromeos/system_logs/single_log_source.cc
@@ -14,6 +14,7 @@ namespace system_logs {
 namespace {
 
 const char kDefaultSystemLogDirPath[] = "/var/log";
+const int kMaxNumAllowedLogRotationsDuringFileRead = 3;

[afakhry, 2017/06/13 17:39:21]

Nit: constexpr for both.

[Simon Que, 2017/06/13 20:54:57]

Done.

 
 // Converts a logs source type to the corresponding file path, relative to the
 // base system log directory path. In the future, if non-file source types are
@@ -30,6 +31,15 @@ base::FilePath GetLogFileSourceRelativeFilePath(
   return base::FilePath();
 }
 
+// Returns the inode value of file at |path|, or 0 if it doesn't exist or is
+// otherwise unable to be accessed for file system info.
+ino_t GetInodeValue(const base::FilePath& path) {
+  struct stat file_stats;
+  if (stat(path.value().c_str(), &file_stats) != 0)
+    return 0;
+  return file_stats.st_ino;
+}
+
 // Attempts to store a string |value| in |*response| under |key|. If there is
 // already a string in |*response| under |key|, appends |value| to the existing
 // string value.
@@ -49,6 +59,7 @@ SingleLogSource::SingleLogSource(SupportedSource source)
     : SystemLogsSource(GetLogFileSourceRelativeFilePath(source).value()),
       log_file_dir_path_(kDefaultSystemLogDirPath),
       num_bytes_read_(0),
+      file_inode_(0),
       weak_ptr_factory_(this) {}
 
 SingleLogSource::~SingleLogSource() {}
@@ -62,17 +73,24 @@ void SingleLogSource::Fetch(const SysLogsSourceCallback& callback) {
       FROM_HERE,
       base::TaskTraits(base::MayBlock(), base::TaskPriority::BACKGROUND),
       base::Bind(&SingleLogSource::ReadFile, weak_ptr_factory_.GetWeakPtr(),
-                 response),
+                 kMaxNumAllowedLogRotationsDuringFileRead, response),
       base::Bind(callback, base::Owned(response)));
 }
 
-void SingleLogSource::ReadFile(SystemLogsResponse* result) {
+base::FilePath SingleLogSource::GetLogFilePath() const {
+  return base::FilePath(log_file_dir_path_).Append(source_name());
+}
+
+void SingleLogSource::ReadFile(size_t num_rotations_allowed,
+                               SystemLogsResponse* result) {
   // Attempt to open the file if it was not previously opened.
   if (!file_.IsValid()) {
-    file_.Initialize(base::FilePath(log_file_dir_path_).Append(source_name()),
+    file_.Initialize(GetLogFilePath(),
                      base::File::FLAG_OPEN | base::File::FLAG_READ);
     if (!file_.IsValid())
       return;
+
+    file_inode_ = GetInodeValue(GetLogFilePath());
   }
 
   // Check for file size reset.
@@ -89,8 +107,18 @@ void SingleLogSource::ReadFile(SystemLogsResponse* result) {
   size_t size_read = file_.ReadAtCurrentPos(&result_string[0], size_to_read);
   result_string.resize(size_read);
 
-  // The reader may only read complete lines.
-  if (result_string.empty() || result_string.back() != '\n') {
+  bool file_was_rotated = false;
+  ino_t new_inode = GetInodeValue(GetLogFilePath());
+  if (new_inode != file_inode_)
+    file_was_rotated = true;

[afakhry, 2017/06/13 17:39:21]

Nit: replace all the above with:

const bool file_was_rotated = 
    file_inode_ != GetInodeValue(GetLogFilePath());

[Simon Que, 2017/06/13 20:54:57]

Done.

+
+  bool handle_file_rotation = file_was_rotated && num_rotations_allowed > 0;

[afakhry, 2017/06/13 17:39:21]

Nit: const bool should_handle_file_rotation = ...

[Simon Que, 2017/06/13 20:54:57]

Done.

+
+  // The reader may only read complete lines. The exception is when there is a
+  // rotation, in which case all the remaining contents of the old log file
+  // should be read before moving on to read the new log file.
+  if ((result_string.empty() || result_string.back() != '\n') &&
+      !handle_file_rotation) {
     // If an incomplete line was read, return only the part that includes whole
     // lines.
     size_t last_newline_pos = result_string.find_last_of('\n');
@@ -114,6 +142,15 @@ void SingleLogSource::ReadFile(SystemLogsResponse* result) {
   // Pass it back to the callback.
   AppendToSystemLogsResponse(result, source_name(),
                              anonymizer_.Anonymize(result_string));
+
+  // If the file was rotated, close the file handle and call this function
+  // again, to read from the new file.
+  if (handle_file_rotation) {
+    file_.Close();
+    num_bytes_read_ = 0;
+    file_inode_ = 0;
+    ReadFile(num_rotations_allowed - 1, result);
+  }
 }
 
 }  // namespace system_logs