Browse Source

[1330] Sorted out clearing up after exception is thrown

Stephen Morris 13 years ago
parent
commit
81b1ba0e9c

+ 1 - 2
src/lib/datasrc/database.h

@@ -319,8 +319,7 @@ public:
      *
      * \return Newly created iterator context. Must not be NULL.
      */
-    virtual IteratorContextPtr getDiffs(int id, uint32_t start, uint32_t end)
-                                        const = 0;
+    virtual IteratorContextPtr getDiffs(int id, int start, int end) const = 0;
 
     /// Start a transaction for updating a zone.
     ///

+ 17 - 12
src/lib/datasrc/sqlite3_accessor.cc

@@ -98,7 +98,7 @@ const char* const text_statements[NUM_STATEMENTS] = {
     "SELECT id FROM diffs "     // LOW_DIFF_ID
         "WHERE zone_id=?1 AND version=?2 and OPERATION=0 "
         "ORDER BY id ASC LIMIT 1",
-    "SELECT id FROM diffs "     // LOW_DIFF_ID
+    "SELECT id FROM diffs "     // HIGH_DIFF_ID
         "WHERE zone_id=?1 AND version=?2 and OPERATION=1 "
         "ORDER BY id DESC LIMIT 1",
     "SELECT name, rrtype, ttl, rdata FROM diffs "   // DIFFS
@@ -605,9 +605,14 @@ public:
                 int id, int start, int end) :
         accessor_(accessor)
     {
-        int low_id = findIndex(LOW_DIFF_ID, id, start);
-        int high_id = findIndex(HIGH_DIFF_ID, id, end);
-        std::cout << "Low index is " << low_id << ", high index is " << high_id << "\n";
+        try {
+            int low_id = findIndex(LOW_DIFF_ID, id, start);
+            int high_id = findIndex(HIGH_DIFF_ID, id, end);
+            std::cout << "Low index is " << low_id << ", high index is " << high_id << "\n";
+        } catch (...) {
+            accessor_->dbparameters_->finalizeStatements();
+            throw;
+        }
     }
 
     virtual ~DiffContext() {}
@@ -686,9 +691,9 @@ private:
             int rc = sqlite3_step(stmt);
             if (rc == SQLITE_DONE) {
 
-                // That was the only data, OK to return. Tidy up as we go,
-                // ignoring any error return.
-                rc = sqlite3_reset(stmt);
+                // All OK, exit with the value.
+                return (result);
+
             } else if (rc == SQLITE_ROW) {
                 isc_throw(DataSourceError, "request to return one value from "
                           "diffs table for serial " << serial << " (zone ID " <<
@@ -701,11 +706,11 @@ private:
                       " for zone ID " << zone_id);
         }
 
-        if (rc != SQLITE_OK) {
-            isc_throw(DataSourceError, "could not get data from diffs table: " <<
-                      sqlite3_errmsg(accessor_->dbparameters_->db_));
-        }
+        // We get here on an error.
+        isc_throw(DataSourceError, "could not get data from diffs table: " <<
+                  sqlite3_errmsg(accessor_->dbparameters_->db_));
 
+        // Keep the compiler happy with a return value.
         return (result);
     }
 
@@ -716,7 +721,7 @@ private:
 // ... and return the iterator
 
 DatabaseAccessor::IteratorContextPtr
-SQLite3Accessor::getDiffs(int id, uint32_t start, uint32_t end) const {
+SQLite3Accessor::getDiffs(int id, int start, int end) const {
     return (IteratorContextPtr(new DiffContext(shared_from_this(), id, start,
                                end)));
 }

+ 1 - 1
src/lib/datasrc/sqlite3_accessor.h

@@ -157,7 +157,7 @@ public:
      *
      * \return Iterator containing difference records.
      */
-    virtual IteratorContextPtr getDiffs(int id, uint32_t start, uint32_t end) const;
+    virtual IteratorContextPtr getDiffs(int id, int start, int end) const;
                                         
 
 

+ 1 - 1
src/lib/datasrc/tests/database_unittest.cc

@@ -252,7 +252,7 @@ public:
                   "This database datasource can't be iterated");
     }
 
-    virtual IteratorContextPtr getDiffs(int, uint32_t, uint32_t) const {
+    virtual IteratorContextPtr getDiffs(int, int, int) const {
         isc_throw(isc::NotImplemented,
                   "This database datasource can't be iterated");
     }

+ 10 - 2
src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -220,9 +220,17 @@ TEST_F(SQLite3AccessorTest, diffIteratorNoVersion) {
     const std::pair<bool, int> zone_info(accessor->getZone("example.org."));
     ASSERT_TRUE(zone_info.first);
 
-    // Get the iterator context.  Difference of version 1 does not exist.
-    EXPECT_THROW(accessor->getDiffs(zone_info.second, 1U, 1234U),
+    // Get the iterator context.  Difference of version 1 does not exist, so
+    // this should throw an exception.
+    EXPECT_THROW(accessor->getDiffs(zone_info.second, 1, 1234),
+                 isc::datasrc::NoSuchSerial);
+
+    // Check that an invalid high version number also throws an exception.
+    EXPECT_THROW(accessor->getDiffs(zone_info.second, 1231, 2234),
                  NoSuchSerial);
+
+
+    // Get the iterator context
 }
 
 TEST(SQLite3Open, getDBNameExample2) {