make CanonicalizePath report an error on empty path

Fixes part of issue 121, but the fix exposed a further issue.

7 files changed, 57 insertions, 19 deletions

diff --git a/src/graph.cc b/src/graph.cc
index 16299c0..0f16519 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -178,7 +178,8 @@ bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface,
   for (vector<StringPiece>::iterator i = makefile.ins_.begin();
        i != makefile.ins_.end(); ++i, ++implicit_dep) {
     string path(i->str_, i->len_);
-    CanonicalizePath(&path);
+    if (!CanonicalizePath(&path, err))
+      return false;
 
     Node* node = state->GetNode(path);
     *implicit_dep = node;
diff --git a/src/ninja.cc b/src/ninja.cc
index c70aa43..6dc2766 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -116,7 +116,8 @@ struct RealFileReader : public ManifestParser::FileReader {
 bool RebuildManifest(State* state, const BuildConfig& config,
                      const char* input_file, string* err) {
   string path = input_file;
-  CanonicalizePath(&path);
+  if (!CanonicalizePath(&path, err))
+    return false;
   Node* node = state->LookupNode(path);
   if (!node)
     return false;
@@ -139,7 +140,8 @@ bool CollectTargetsFromArgs(State* state, int argc, char* argv[],
   } else {
     for (int i = 0; i < argc; ++i) {
       string path = argv[i];
-      CanonicalizePath(&path);
+      if (!CanonicalizePath(&path, err))
+        return false;
 
       // Special syntax: "foo.cc^" means "the first output of foo.cc".
       bool first_dependent = false;
diff --git a/src/parsers.cc b/src/parsers.cc
index 567be6b..9ed2938 100644
--- a/src/parsers.cc
+++ b/src/parsers.cc
@@ -462,7 +462,8 @@ bool ManifestParser::ParseDefaults(string* err) {
     if (!eval.Parse(target, &eval_err))
       return tokenizer_.Error(eval_err, err);
     string path = eval.Evaluate(env_);
-    CanonicalizePath(&path);
+    if (!CanonicalizePath(&path, &eval_err))
+      return tokenizer_.Error(eval_err, err);
     if (!state_->AddDefault(path, &eval_err))
       return tokenizer_.Error(eval_err, err);
   } while (tokenizer_.ReadIdent(&target));
@@ -566,7 +567,8 @@ bool ManifestParser::ParseEdge(string* err) {
       if (!eval.Parse(*i, &eval_err))
         return tokenizer_.Error(eval_err, err);
       string path = eval.Evaluate(env);
-      CanonicalizePath(&path);
+      if (!CanonicalizePath(&path, &eval_err))
+        return tokenizer_.Error(eval_err, err);
       *i = path;
     }
   }
diff --git a/src/parsers_test.cc b/src/parsers_test.cc
index c1c33a2..a99b510 100644
--- a/src/parsers_test.cc
+++ b/src/parsers_test.cc
@@ -359,6 +359,26 @@ TEST_F(ParserTest, Errors) {
                               &err));
     EXPECT_EQ("line 4, col 10: expected newline, got ':'", err);
   }
+
+  {
+    State state;
+    ManifestParser parser(&state, NULL);
+    string err;
+    EXPECT_FALSE(parser.Parse("default $a\n", &err));
+    EXPECT_EQ("line 1, col 9: empty path", err);
+  }
+
+  {
+    State state;
+    ManifestParser parser(&state, NULL);
+    string err;
+    EXPECT_FALSE(parser.Parse("rule r\n"
+                              "  command = r\n"
+                              "build $a: r $c\n", &err));
+    // XXX the line number is wrong; we should evaluate paths in ParseEdge
+    // as we see them, not after we've read them all!
+    EXPECT_EQ("line 4, col 1: empty path", err);
+  }
 }
 
 TEST_F(ParserTest, MultipleOutputs)
diff --git a/src/util.cc b/src/util.cc
index 0ad470e..e2b20d6 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -64,10 +64,15 @@ void Error(const char* msg, ...) {
   fprintf(stderr, "\n");
 }
 
-void CanonicalizePath(string* path) {
+bool CanonicalizePath(string* path, string* err) {
   // WARNING: this function is performance-critical; please benchmark
   // any changes you make to it.
 
+  if (path->empty()) {
+    *err = "empty path";
+    return false;
+  }
+
   const int kMaxPathComponents = 30;
   char* components[kMaxPathComponents];
   int component_count = 0;
@@ -120,6 +125,7 @@ void CanonicalizePath(string* path) {
   }
 
   path->resize(dst - path->c_str() - 1);
+  return true;
 }
 
 int MakeDir(const string& path) {
diff --git a/src/util.h b/src/util.h
index 58a0a3a..c9e06f4 100644
--- a/src/util.h
+++ b/src/util.h
@@ -31,7 +31,7 @@ void Warning(const char* msg, ...);
 void Error(const char* msg, ...);
 
 /// Canonicalize a path like "foo/../bar.h" into just "bar.h".
-void CanonicalizePath(string* path);
+bool CanonicalizePath(string* path, string* err);
 
 /// Create a directory (mode 0777 on Unix).
 /// Portability abstraction.
diff --git a/src/util_test.cc b/src/util_test.cc
index 97a236f..8c3d023 100644
--- a/src/util_test.cc
+++ b/src/util_test.cc
@@ -17,52 +17,59 @@
 #include "test.h"
 
 TEST(CanonicalizePath, PathSamples) {
-  std::string path = "foo.h";
-  CanonicalizePath(&path);
+  string path;
+  string err;
+
+  EXPECT_FALSE(CanonicalizePath(&path, &err));
+  EXPECT_EQ("empty path", err);
+
+  path = "foo.h"; err = "";
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("foo.h", path);
 
   path = "./foo.h";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("foo.h", path);
 
   path = "./foo/./bar.h";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("foo/bar.h", path);
 
   path = "./x/foo/../bar.h";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("x/bar.h", path);
 
   path = "./x/foo/../../bar.h";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("bar.h", path);
 
   path = "foo//bar";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("foo/bar", path);
 
   path = "foo//.//..///bar";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("bar", path);
 
   path = "./x/../foo/../../bar.h";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("../bar.h", path);
 }
 
 TEST(CanonicalizePath, UpDir) {
   std::string path, err;
   path = "../../foo/bar.h";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("../../foo/bar.h", path);
 
   path = "test/../../foo/bar.h";
-  CanonicalizePath(&path);
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("../foo/bar.h", path);
 }
 
 TEST(CanonicalizePath, AbsolutePath) {
   string path = "/usr/include/stdio.h";
-  CanonicalizePath(&path);
+  string err;
+  EXPECT_TRUE(CanonicalizePath(&path, &err));
   EXPECT_EQ("/usr/include/stdio.h", path);
 }