diff options
author | Evan Martin <martine@danga.com> | 2011-10-06 00:51:06 (GMT) |
---|---|---|
committer | Evan Martin <martine@danga.com> | 2011-10-06 00:51:06 (GMT) |
commit | e54d2b6c8bbb78b238160c7f31ae8515c15183d9 (patch) | |
tree | 2566a133dfda2675b55c896dbf22f95a7bbfced5 /src | |
parent | 06ab305be33ff4899016c746ee8a3557291ebe09 (diff) | |
download | Ninja-e54d2b6c8bbb78b238160c7f31ae8515c15183d9.zip Ninja-e54d2b6c8bbb78b238160c7f31ae8515c15183d9.tar.gz Ninja-e54d2b6c8bbb78b238160c7f31ae8515c15183d9.tar.bz2 |
make CanonicalizePath report an error on empty path
Fixes part of issue 121, but the fix exposed a further issue.
Diffstat (limited to 'src')
-rw-r--r-- | src/graph.cc | 3 | ||||
-rw-r--r-- | src/ninja.cc | 6 | ||||
-rw-r--r-- | src/parsers.cc | 6 | ||||
-rw-r--r-- | src/parsers_test.cc | 20 | ||||
-rw-r--r-- | src/util.cc | 8 | ||||
-rw-r--r-- | src/util.h | 2 | ||||
-rw-r--r-- | src/util_test.cc | 31 |
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) { @@ -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); } |