Merge pull request #612 from maximuska/proposed/deps_log_write_errors_checking

Proposed/deps log write errors checking

3 files changed, 56 insertions, 20 deletions

diff --git a/src/build.cc b/src/build.cc
index 143aeb2..67e4634 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -639,7 +639,10 @@ bool Builder::Build(string* err) {
       }
 
       --pending_commands;
-      FinishCommand(&result);
+      if (!FinishCommand(&result, err)) {
+        status_->BuildFinished();
+        return false;
+      }
 
       if (!result.success()) {
         if (failures_allowed)
@@ -702,7 +705,7 @@ bool Builder::StartEdge(Edge* edge, string* err) {
   return true;
 }
 
-void Builder::FinishCommand(CommandRunner::Result* result) {
+bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
   METRIC_RECORD("FinishCommand");
 
   Edge* edge = result->edge;
@@ -731,7 +734,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) {
 
   // The rest of this function only applies to successful commands.
   if (!result->success())
-    return;
+    return true;
 
   // Restat the edge outputs, if necessary.
   TimeStamp restat_mtime = 0;
@@ -789,9 +792,12 @@ void Builder::FinishCommand(CommandRunner::Result* result) {
     assert(edge->outputs_.size() == 1 && "should have been rejected by parser");
     Node* out = edge->outputs_[0];
     TimeStamp deps_mtime = disk_interface_->Stat(out->path());
-    scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes);
+    if (!scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes)) {
+      *err = string("Error writing to deps log: ") + strerror(errno);
+      return false;
+    }
   }
-
+  return true;
 }
 
 bool Builder::ExtractDeps(CommandRunner::Result* result,
diff --git a/src/build.h b/src/build.h
index 2715c0c..33df7d0 100644
--- a/src/build.h
+++ b/src/build.h
@@ -163,7 +163,10 @@ struct Builder {
   bool Build(string* err);
 
   bool StartEdge(Edge* edge, string* err);
-  void FinishCommand(CommandRunner::Result* result);
+
+  /// Update status ninja logs following a command termination.
+  /// @return false if the build can not proceed further due to a fatal error.
+  bool FinishCommand(CommandRunner::Result* result, string* err);
 
   /// Used for tests.
   void SetBuildLog(BuildLog* log) {
diff --git a/src/deps_log.cc b/src/deps_log.cc
index 63e0d95..2c4e3c2 100644
--- a/src/deps_log.cc
+++ b/src/deps_log.cc
@@ -37,6 +37,9 @@ const int kCurrentVersion = 1;
 // buffer after every record to make sure records aren't written partially.
 const int kMaxBufferSize = 1 << 15;
 
+// Record size is currently limited to 15 bit
+const size_t kMaxRecordSize = (1 << 15) - 1;
+
 DepsLog::~DepsLog() {
   Close();
 }
@@ -69,8 +72,10 @@ bool DepsLog::OpenForWrite(const string& path, string* err) {
       return false;
     }
   }
-  fflush(file_);
-
+  if (fflush(file_) != 0) {
+    *err = strerror(errno);
+    return false;
+  }
   return true;
 }
 
@@ -87,12 +92,14 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime,
 
   // Assign ids to all nodes that are missing one.
   if (node->id() < 0) {
-    RecordId(node);
+    if (!RecordId(node))
+      return false;
     made_change = true;
   }
   for (int i = 0; i < node_count; ++i) {
     if (nodes[i]->id() < 0) {
-      RecordId(nodes[i]);
+      if (!RecordId(nodes[i]))
+        return false;
       made_change = true;
     }
   }
@@ -119,18 +126,28 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime,
     return true;
 
   // Update on-disk representation.
-  uint16_t size = 4 * (1 + 1 + (uint16_t)node_count);
+  size_t size = 4 * (1 + 1 + (uint16_t)node_count);
+  if (size > kMaxRecordSize) {
+    errno = ERANGE;
+    return false;
+  }
   size |= 0x8000;  // Deps record: set high bit.
-  fwrite(&size, 2, 1, file_);
+  uint16_t size16 = (uint16_t)size;
+  if (fwrite(&size16, 2, 1, file_) < 1)
+    return false;
   int id = node->id();
-  fwrite(&id, 4, 1, file_);
+  if (fwrite(&id, 4, 1, file_) < 1)
+    return false;
   int timestamp = mtime;
-  fwrite(&timestamp, 4, 1, file_);
+  if (fwrite(&timestamp, 4, 1, file_) < 1)
+    return false;
   for (int i = 0; i < node_count; ++i) {
     id = nodes[i]->id();
-    fwrite(&id, 4, 1, file_);
+    if (fwrite(&id, 4, 1, file_) < 1)
+      return false;
   }
-  fflush(file_);
+  if (fflush(file_) != 0)
+      return false;
 
   // Update in-memory representation.
   Deps* deps = new Deps(mtime, node_count);
@@ -323,10 +340,20 @@ bool DepsLog::UpdateDeps(int out_id, Deps* deps) {
 }
 
 bool DepsLog::RecordId(Node* node) {
-  uint16_t size = (uint16_t)node->path().size();
-  fwrite(&size, 2, 1, file_);
-  fwrite(node->path().data(), node->path().size(), 1, file_);
-  fflush(file_);
+  size_t size = node->path().size();
+  if (size > kMaxRecordSize) {
+    errno = ERANGE;
+    return false;
+  }
+  uint16_t size16 = (uint16_t)size;
+  if (fwrite(&size16, 2, 1, file_) < 1)
+    return false;
+  if (fwrite(node->path().data(), node->path().size(), 1, file_) < 1) {
+    assert(node->path().size() > 0);
+    return false;
+  }
+  if (fflush(file_) != 0)
+    return false;
 
   node->set_id(nodes_.size());
   nodes_.push_back(node);