From: Michal 'vorner' Vaner <michal.vaner@nic.cz>
Date: Mon, 3 Dec 2012 12:11:16 +0000 (+0100)
Subject: [2377] Handle errors on file open
X-Git-Tag: bind10-1.0.0-beta-release~24^2~56
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cbffb4ac5bf27888553a102ac6ee2a1fbf07642b;p=thirdparty%2Fkea.git

[2377] Handle errors on file open
---

diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 06cf7c55d1..1acb14b552 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -38,12 +38,17 @@ public:
         zone_class_(zone_class),
         callbacks_(callbacks),
         add_callback_(add_callback),
-        options_(options)
-    {
-        string errors;
-        if (!lexer_.pushSource(master_file, &errors)) {
-            // TODO: Handle somehow.
-            assert(0);
+        options_(options),
+        master_file_(master_file),
+        initialized_(false),
+        ok_(true)
+    {}
+
+    void pushSource(const std::string& filename) {
+        std::string error;
+        if (!lexer_.pushSource(filename.c_str(), &error)) {
+            ok_ = false;
+            callbacks_.error("", 0, error);
         }
     }
 
@@ -53,8 +58,11 @@ public:
     }
 
     bool loadIncremental(size_t count_limit) {
+        if (!initialized_) {
+            pushSource(master_file_);
+        }
         size_t count = 0;
-        while (count < count_limit) {
+        while (ok_ && count < count_limit) {
             // Skip all EOLNs (empty lines) and finish on EOF
             bool empty = true;
             do {
@@ -93,7 +101,8 @@ public:
                 ++count;
             };
         }
-        return (false);
+        // When there was a fatal error and ok is false, we say we are done.
+        return (!ok_);
     }
 
 private:
@@ -103,6 +112,9 @@ private:
     MasterLoaderCallbacks callbacks_;
     AddRRCallback add_callback_;
     MasterLoader::Options options_;
+    const std::string master_file_;
+    bool initialized_;
+    bool ok_;
 };
 
 MasterLoader::MasterLoader(const char* master_file,
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index 7b275792d7..e7ebdee6fc 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -47,7 +47,7 @@ public:
                   const std::string reason)
     {
         std::stringstream ss;
-        ss << file << line << reason;
+        ss << reason << " [" << file << ":" << line << "]";
         if (error) {
             errors_.push_back(ss.str());
         } else {
@@ -97,9 +97,7 @@ public:
 // Test simple loading. The zone file contains no tricky things, and nothing is
 // omitted. No RRset contains more than one RR Also no errors or warnings.
 TEST_F(MasterLoaderTest, basicLoad) {
-    setLoader("example.org",
-              Name("example.org."),
-              RRClass::IN(),
+    setLoader("example.org", Name("example.org."), RRClass::IN(),
               MasterLoader::MANY_ERRORS);
 
     loader_->load();
@@ -112,3 +110,22 @@ TEST_F(MasterLoaderTest, basicLoad) {
     checkRR("example.org", RRType::NS(), "ns1.example.org.");
     checkRR("www.example.org", RRType::A(), "192.0.2.1");
 }
+
+// Try loading from file that doesn't exist. There should be single error
+// saying so.
+TEST_F(MasterLoaderTest, invalidFile) {
+    setLoader("This file doesn't exist at all",
+              Name("exmaple.org."), RRClass::IN(), MasterLoader::MANY_ERRORS);
+
+    // Nothing yet. The loader is dormant until invoked.
+    // Is it really what we want?
+    EXPECT_TRUE(errors_.empty());
+
+    loader_->load();
+
+    EXPECT_TRUE(warnings_.empty());
+    EXPECT_TRUE(rrsets_.empty());
+    ASSERT_EQ(1, errors_.size());
+    EXPECT_EQ(0, errors_[0].find("Error opening the input source file: ")) <<
+        "Different error: " << errors_[0];
+}