Expose more details in FileReader::ReadFile signature

Return a status so callers can distinguish a missing file from an empty
file.  This allows our VirtualFileSystem test infrastructure to report
as missing any file for which it has no entry.

Author: bradking

src/build.cc

@@ -871,9 +871,17 @@ bool Builder::ExtractDeps(CommandRunner::Result* result,
       return false;
     }
 
-    string content = disk_interface_->ReadFile(depfile, err);
-    if (!err->empty())
+    // Read depfile content.  Treat a missing depfile as empty.
+    string content;
+    switch (disk_interface_->ReadFile(depfile, &content, err)) {
+    case DiskInterface::Okay:
+      break;
+    case DiskInterface::NotFound:
+      err->clear();
+      break;
+    case DiskInterface::OtherError:
       return false;
+    }
     if (content.empty())
       return true;
 

src/disk_interface.cc

@@ -229,14 +229,14 @@ bool RealDiskInterface::MakeDir(const string& path) {
   return true;
 }
 
-string RealDiskInterface::ReadFile(const string& path, string* err) {
-  string contents;
-  int ret = ::ReadFile(path, &contents, err);
-  if (ret == -ENOENT) {
-    // Swallow ENOENT.
-    err->clear();
+FileReader::Status RealDiskInterface::ReadFile(const string& path,
+                                               string* contents,
+                                               string* err) {
+  switch (::ReadFile(path, contents, err)) {
+  case 0:       return Okay;
+  case -ENOENT: return NotFound;
+  default:      return OtherError;
   }
-  return contents;
 }
 
 int RealDiskInterface::RemoveFile(const string& path) {

src/disk_interface.h

@@ -26,8 +26,17 @@ using namespace std;
 struct FileReader {
   virtual ~FileReader() {}
 
-  /// Read a file to a string.  Fill in |err| on error.
-  virtual string ReadFile(const string& path, string* err) = 0;
+  /// Result of ReadFile.
+  enum Status {
+    Okay,
+    NotFound,
+    OtherError
+  };
+
+  /// Read and store in given string.  On success, return Okay.
+  /// On error, return another Status and fill |err|.
+  virtual Status ReadFile(const string& path, string* contents,
+                          string* err) = 0;
 };
 
 /// Interface for accessing the disk.
@@ -69,7 +78,7 @@ struct RealDiskInterface : public DiskInterface {
   virtual TimeStamp Stat(const string& path, string* err) const;
   virtual bool MakeDir(const string& path);
   virtual bool WriteFile(const string& path, const string& contents);
-  virtual string ReadFile(const string& path, string* err);
+  virtual Status ReadFile(const string& path, string* contents, string* err);
   virtual int RemoveFile(const string& path);
 
   /// Whether stat information can be cached.  Only has an effect on Windows.

src/disk_interface_test.cc

@@ -157,8 +157,12 @@ TEST_F(DiskInterfaceTest, StatCache) {
 
 TEST_F(DiskInterfaceTest, ReadFile) {
   string err;
-  EXPECT_EQ("", disk_.ReadFile("foobar", &err));
-  EXPECT_EQ("", err);
+  std::string content;
+  ASSERT_EQ(DiskInterface::NotFound,
+            disk_.ReadFile("foobar", &content, &err));
+  EXPECT_EQ("", content);
+  EXPECT_NE("", err); // actual value is platform-specific
+  err.clear();
 
   const char* kTestFile = "testfile";
   FILE* f = fopen(kTestFile, "wb");
@@ -167,7 +171,9 @@ TEST_F(DiskInterfaceTest, ReadFile) {
   fprintf(f, "%s", kTestContent);
   ASSERT_EQ(0, fclose(f));
 
-  EXPECT_EQ(kTestContent, disk_.ReadFile(kTestFile, &err));
+  ASSERT_EQ(DiskInterface::Okay,
+            disk_.ReadFile(kTestFile, &content, &err));
+  EXPECT_EQ(kTestContent, content);
   EXPECT_EQ("", err);
 }
 
@@ -208,9 +214,9 @@ struct StatTest : public StateTestWithBuiltinRules,
     assert(false);
     return false;
   }
-  virtual string ReadFile(const string& path, string* err) {
+  virtual Status ReadFile(const string& path, string* contents, string* err) {
     assert(false);
-    return "";
+    return NotFound;
   }
   virtual int RemoveFile(const string& path) {
     assert(false);

src/graph.cc

@@ -395,8 +395,15 @@ bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) {
 bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path,
                                     string* err) {
   METRIC_RECORD("depfile load");
-  string content = disk_interface_->ReadFile(path, err);
-  if (!err->empty()) {
+  // Read depfile content.  Treat a missing depfile as empty.
+  string content;
+  switch (disk_interface_->ReadFile(path, &content, err)) {
+  case DiskInterface::Okay:
+    break;
+  case DiskInterface::NotFound:
+    err->clear();
+    break;
+  case DiskInterface::OtherError:
     *err = "loading '" + path + "': " + *err;
     return false;
   }

src/test.cc

@@ -164,12 +164,17 @@ bool VirtualFileSystem::MakeDir(const string& path) {
   return true;  // success
 }
 
-string VirtualFileSystem::ReadFile(const string& path, string* err) {
+FileReader::Status VirtualFileSystem::ReadFile(const string& path,
+                                               string* contents,
+                                               string* err) {
   files_read_.push_back(path);
   FileMap::iterator i = files_.find(path);
-  if (i != files_.end())
-    return i->second.contents;
-  return "";
+  if (i != files_.end()) {
+    *contents = i->second.contents;
+    return Okay;
+  }
+  *err = strerror(ENOENT);
+  return NotFound;
 }
 
 int VirtualFileSystem::RemoveFile(const string& path) {

src/test.h

@@ -145,7 +145,7 @@ struct VirtualFileSystem : public DiskInterface {
   virtual TimeStamp Stat(const string& path, string* err) const;
   virtual bool WriteFile(const string& path, const string& contents);
   virtual bool MakeDir(const string& path);
-  virtual string ReadFile(const string& path, string* err);
+  virtual Status ReadFile(const string& path, string* contents, string* err);
   virtual int RemoveFile(const string& path);
 
   /// An entry for a single in-memory file.