[1183] now it's been unified, we can make compiler enforce array size

that makes a number of tests unnecessary (as well as impossible)

src/lib/datasrc/database.cc

@@ -191,7 +191,7 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
     }
 
     std::string columns[DatabaseAccessor::COLUMN_COUNT];
-    while (context->getNext(columns, DatabaseAccessor::COLUMN_COUNT)) {
+    while (context->getNext(columns)) {
         if (!records_found) {
             records_found = true;
         }
@@ -466,7 +466,7 @@ private:
     // Load next row of data
     void getData() {
         string data[DatabaseAccessor::COLUMN_COUNT];
-        data_ready_ = context_->getNext(data, DatabaseAccessor::COLUMN_COUNT);
+        data_ready_ = context_->getNext(data);
         name_ = data[DatabaseAccessor::NAME_COLUMN];
         rtype_ = data[DatabaseAccessor::TYPE_COLUMN];
         ttl_ = data[DatabaseAccessor::TTL_COLUMN];

src/lib/datasrc/database.h

@@ -48,6 +48,9 @@ namespace datasrc {
  */
 class DatabaseAccessor : boost::noncopyable {
 public:
+    /// The number of fields the columns array passed to getNext should have
+    static const size_t COLUMN_COUNT = 5;
+
     /**
      * \brief Destructor
      *
@@ -119,7 +122,7 @@ public:
          *     exception (or any other in case of derived class) is thrown,
          *     the iterator can't be safely used any more.
          */
-        virtual bool getNext(std::string columns[], size_t column_data) = 0;
+        virtual bool getNext(std::string (&columns)[COLUMN_COUNT]) = 0;
     };
 
     typedef boost::shared_ptr<IteratorContext> IteratorContextPtr;
@@ -206,9 +209,6 @@ public:
         NAME_COLUMN = 4     ///< The domain name of this RR
     };
 
-    /// The number of fields the columns array passed to getNextRecord should have
-    static const size_t COLUMN_COUNT = 5;
-
     /**
      * \brief Returns a string identifying this dabase backend
      *

src/lib/datasrc/sqlite3_accessor.cc

@@ -394,11 +394,7 @@ public:
         bindName(name);
     }
 
-    bool getNext(std::string data[], size_t size) {
-        if (size != COLUMN_COUNT) {
-            isc_throw(DataSourceError, "getNext received size of " << size <<
-                      ", not " << COLUMN_COUNT);
-        }
+    bool getNext(std::string (&data)[COLUMN_COUNT]) {
         // If there's another row, get it
         int rc(sqlite3_step(statement_));
         if (rc == SQLITE_ROW) {

src/lib/datasrc/tests/database_unittest.cc

@@ -111,7 +111,7 @@ private:
             }
         }
 
-        virtual bool getNext(std::string columns[], size_t column_count) {
+        virtual bool getNext(std::string (&columns)[COLUMN_COUNT]) {
             if (searched_name_ == "dsexception.in.getnext.") {
                 isc_throw(DataSourceError, "datasource exception on getnextrecord");
             } else if (searched_name_ == "iscexception.in.getnext.") {
@@ -120,11 +120,8 @@ private:
                 throw std::exception();
             }
 
-            if (column_count != DatabaseAccessor::COLUMN_COUNT) {
-                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-            }
             if (cur_record_ < cur_name.size()) {
-                for (size_t i = 0; i < column_count; ++i) {
+                for (size_t i = 0; i < COLUMN_COUNT; ++i) {
                     columns[i] = cur_name[cur_record_][i];
                 }
                 cur_record_++;
@@ -147,10 +144,7 @@ private:
         MockIteratorContext() :
             step(0)
         { }
-        virtual bool getNext(string data[], size_t size) {
-            if (size != DatabaseAccessor::COLUMN_COUNT) {
-                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-            }
+        virtual bool getNext(string (&data)[COLUMN_COUNT]) {
             switch (step ++) {
                 case 0:
                     data[DatabaseAccessor::NAME_COLUMN] = "example.org";
@@ -193,10 +187,7 @@ private:
     };
     class EmptyIteratorContext : public IteratorContext {
     public:
-        virtual bool getNext(string[], size_t size) {
-            if (size != DatabaseAccessor::COLUMN_COUNT) {
-                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-            }
+        virtual bool getNext(string(&)[COLUMN_COUNT]) {
             return (false);
         }
     };
@@ -207,10 +198,7 @@ private:
         BadIteratorContext() :
             step(0)
         { }
-        virtual bool getNext(string data[], size_t size) {
-            if (size != DatabaseAccessor::COLUMN_COUNT) {
-                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-            }
+        virtual bool getNext(string (&data)[COLUMN_COUNT]) {
             switch (step ++) {
                 case 0:
                     data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -117,14 +117,14 @@ TEST_F(SQLite3Access, iterator) {
     const size_t size(5);
     std::string data[size];
     // Get and check the first and only record
-    EXPECT_TRUE(context->getNext(data, size));
+    EXPECT_TRUE(context->getNext(data));
     EXPECT_EQ("example2.com.", data[4]);
     EXPECT_EQ("SOA", data[0]);
     EXPECT_EQ("master.example2.com. admin.example2.com. "
               "1234 3600 1800 2419200 7200", data[3]);
     EXPECT_EQ("3600", data[1]);
     // Check there's no other
-    EXPECT_FALSE(context->getNext(data, size));
+    EXPECT_FALSE(context->getNext(data));
 }
 
 TEST_F(SQLite3Access, iteratorColumnCount) {
@@ -137,11 +137,8 @@ TEST_F(SQLite3Access, iteratorColumnCount) {
     ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
               context);
 
-    EXPECT_THROW(context->getNext(NULL, 0), DataSourceError);
-    std::string data[6];
-    EXPECT_THROW(context->getNext(data, 4), DataSourceError);
-    EXPECT_THROW(context->getNext(data, 6), DataSourceError);
-    EXPECT_NO_THROW(context->getNext(data, 5));
+    std::string data[DatabaseAccessor::COLUMN_COUNT];
+    EXPECT_NO_THROW(context->getNext(data));
 }
 
 TEST(SQLite3Open, getDBNameExample2) {
@@ -183,89 +180,85 @@ TEST_F(SQLite3Access, getRecords) {
 
     // TODO: can't do this anymore
     // without search, getNext() should return false
-    //EXPECT_FALSE(context->getNext(columns, column_count));
+    //EXPECT_FALSE(context->getNext(columns));
     //checkRecordRow(columns, "", "", "", "", "");
 
     DatabaseAccessor::IteratorContextPtr
         context(db->getRecords(Name("foo.bar"), 1));
     ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
               context);
-    EXPECT_FALSE(context->getNext(columns, column_count));
+    EXPECT_FALSE(context->getNext(columns));
     checkRecordRow(columns, "", "", "", "", "");
 
     // TODO can't pass incomplete name anymore
     //context = db->getRecords(Name(""), zone_id);
-    //EXPECT_FALSE(context->getNext(columns, column_count));
+    //EXPECT_FALSE(context->getNext(columns));
     //checkRecordRow(columns, "", "", "", "", "");
 
-    // Should error on a bad number of columns
-    EXPECT_THROW(context->getNext(columns, 4), DataSourceError);
-    EXPECT_THROW(context->getNext(columns, 6), DataSourceError);
-
     // now try some real searches
     context = db->getRecords(Name("foo.example.com."), zone_id);
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "CNAME", "3600", "",
                    "cnametest.example.org.", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "CNAME",
                    "CNAME 5 3 3600 20100322084538 20100220084538 33495 "
                    "example.com. FAKEFAKEFAKEFAKE", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "NSEC", "7200", "",
                    "mail.example.com. CNAME RRSIG NSEC", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "7200", "NSEC",
                    "NSEC 5 3 7200 20100322084538 20100220084538 33495 "
                    "example.com. FAKEFAKEFAKEFAKE", "");
-    EXPECT_FALSE(context->getNext(columns, column_count));
+    EXPECT_FALSE(context->getNext(columns));
     // with no more records, the array should not have been modified
     checkRecordRow(columns, "RRSIG", "7200", "NSEC",
                    "NSEC 5 3 7200 20100322084538 20100220084538 33495 "
                    "example.com. FAKEFAKEFAKEFAKE", "");
 
     context = db->getRecords(Name("example.com."), zone_id);
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "SOA", "3600", "",
                    "master.example.com. admin.example.com. "
                    "1234 3600 1800 2419200 7200", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "SOA",
                    "SOA 5 2 3600 20100322084538 20100220084538 "
                    "33495 example.com. FAKEFAKEFAKEFAKE", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "NS", "1200", "", "dns01.example.com.", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "NS", "3600", "", "dns02.example.com.", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "NS", "1800", "", "dns03.example.com.", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "NS",
                    "NS 5 2 3600 20100322084538 20100220084538 "
                    "33495 example.com. FAKEFAKEFAKEFAKE", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "MX", "3600", "", "10 mail.example.com.", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "MX", "3600", "",
                    "20 mail.subzone.example.com.", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "MX",
                    "MX 5 2 3600 20100322084538 20100220084538 "
                    "33495 example.com. FAKEFAKEFAKEFAKE", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "NSEC", "7200", "",
                    "cname-ext.example.com. NS SOA MX RRSIG NSEC DNSKEY", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "7200", "NSEC",
                    "NSEC 5 2 7200 20100322084538 20100220084538 "
                    "33495 example.com. FAKEFAKEFAKEFAKE", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "DNSKEY", "3600", "",
                    "256 3 5 AwEAAcOUBllYc1hf7ND9uDy+Yz1BF3sI0m4q NGV7W"
                    "cTD0WEiuV7IjXgHE36fCmS9QsUxSSOV o1I/FMxI2PJVqTYHkX"
                    "FBS7AzLGsQYMU7UjBZ SotBJ6Imt5pXMu+lEDNy8TOUzG3xm7g"
                    "0qcbW YF6qCEfvZoBtAqi5Rk7Mlrqs8agxYyMx", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "DNSKEY", "3600", "",
                    "257 3 5 AwEAAe5WFbxdCPq2jZrZhlMj7oJdff3W7syJ tbvzg"
                    "62tRx0gkoCDoBI9DPjlOQG0UAbj+xUV 4HQZJStJaZ+fHU5AwV"
@@ -275,15 +268,15 @@ TEST_F(SQLite3Access, getRecords) {
                    "fiHAxFHrkY3t3D5J R9Nsl/7fdRmSznwtcSDgLXBoFEYmw6p86"
                    "Acv RyoYNcL1SXjaKVLG5jyU3UR+LcGZT5t/0xGf oIK/aKwEN"
                    "rsjcKZZj660b1M=", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "DNSKEY",
                    "DNSKEY 5 2 3600 20100322084538 20100220084538 "
                    "4456 example.com. FAKEFAKEFAKEFAKE", "");
-    ASSERT_TRUE(context->getNext(columns, column_count));
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "DNSKEY",
                    "DNSKEY 5 2 3600 20100322084538 20100220084538 "
                    "33495 example.com. FAKEFAKEFAKEFAKE", "");
-    EXPECT_FALSE(context->getNext(columns, column_count));
+    EXPECT_FALSE(context->getNext(columns));
     // getnextrecord returning false should mean array is not altered
     checkRecordRow(columns, "RRSIG", "3600", "DNSKEY",
                    "DNSKEY 5 2 3600 20100322084538 20100220084538 "