btree_damage_visitor stuff.

persistent-data/data-structures/btree_damage_visitor.h

@@ -83,7 +83,16 @@ namespace persistent_data {
 
 		bool visit_leaf(node_location const &loc,
 				btree_detail::node_ref<ValueTraits> const &n) {
-			return check_leaf(loc, n);
+			bool r = check_leaf(loc, n);
+
+			// If anything goes wrong with the checks, we skip
+			// the value visiting.
+			if (!r)
+				return false;
+
+			visit_values(n);
+
+			return true;
 		}
 
 		typedef typename btree<Levels, ValueTraits>::visitor::error_outcome error_outcome;
@@ -94,8 +103,13 @@ namespace persistent_data {
 			return btree<Levels, ValueTraits>::visitor::EXCEPTION_HANDLED;
 		}
 
-
 	private:
+		void visit_values(btree_detail::node_ref<ValueTraits> const &n) {
+			unsigned nr = n.get_nr_entries();
+			for (unsigned i = 0; i < nr; i++)
+				value_visitor_.visit(n.value_at(i));
+		}
+
 		bool check_internal(node_location const &loc,
 				    btree_detail::node_ref<uint64_traits> const &n) {
 			if (!already_visited(n) &&

unit-tests/btree_damage_visitor_t.cc

@@ -17,14 +17,24 @@ using namespace testing;
 namespace {
 	block_address const BLOCK_SIZE = 4096;
 	block_address const NR_BLOCKS = 102400;
+	block_address const SUPERBLOCK = 0;
 
 	struct thing {
-		uint32_t x;
+		thing(/* uint32_t x_ = 0, */ uint64_t y_ = 0)
+			: /* x(x_), */
+			  y(y_) {
+		}
+
+		bool operator ==(thing const &rhs) const {
+			return /* (x == rhs.x) && */ (y == rhs.y);
+		}
+
+		//uint32_t x;
 		uint64_t y;
 	};
 
 	struct thing_disk {
-		le32 x;
+		//le32 x;
 		le64 y;
 	};
 
@@ -34,12 +44,12 @@ namespace {
 		typedef persistent_data::no_op_ref_counter<value_type> ref_counter;
 
 		static void unpack(thing_disk const &disk, thing &value) {
-			value.x = to_cpu<uint32_t>(disk.x);
+			//	value.x = to_cpu<uint32_t>(disk.x);
 			value.y = to_cpu<uint64_t>(disk.y);
 		}
 
 		static void pack(thing const &value, thing_disk &disk) {
-			disk.x = to_disk<le32>(value.x);
+			//	disk.x = to_disk<le32>(value.x);
 			disk.y = to_disk<le64>(value.y);
 		}
 	};
@@ -58,22 +68,74 @@ namespace {
 	public:
 		BTreeDamageVisitorTests()
 			: bm_(create_bm<BLOCK_SIZE>(NR_BLOCKS)),
-			  sm_(new core_map(NR_BLOCKS)),
+			  sm_(setup_core_map()),
 			  tm_(new transaction_manager(bm_, sm_)),
-			  tree_(new btree<2, thing_traits>(tm_, rc_)) {
+			  tree_(new btree<1, thing_traits>(tm_, rc_)) {
 		}
 
-		void zero_block(block_address b) {
+		space_map::ptr setup_core_map() {
+			space_map::ptr sm(new core_map(NR_BLOCKS));
+			sm->inc(SUPERBLOCK);
+			return sm;
+		}
+
+		void commit() {
+			block_manager<>::write_ref superblock(bm_->superblock(SUPERBLOCK));
+		}
+
+		void trash_block(block_address b) {
 			::test::zero_block(bm_, b);
 		}
 
+		void insert_values(unsigned nr) {
+			for (unsigned i = 0; i < nr; i++) {
+				uint64_t key[1] = {i};
+				thing value(i /*, i + 1234 */);
+
+				tree_->insert(key, value);
+			}
+		}
+
+		void expect_no_values() {
+			EXPECT_CALL(value_visitor_, visit(_)).Times(0);
+		}
+
+		void expect_nr_values(unsigned nr) {
+			for (unsigned i = 0; i < nr; i++)
+				EXPECT_CALL(value_visitor_, visit(Eq(thing(i /*, i + 1234 */)))).Times(1);
+		}
+
+		void expect_value(unsigned n) {
+			EXPECT_CALL(value_visitor_, visit(Eq(thing(n /*, n + 1234 */)))).Times(1);
+		}
+
+		void expect_no_damage() {
+			EXPECT_CALL(damage_visitor_, visit(_)).Times(0);
+		}
+
+		void expect_damage(unsigned level, range<uint64_t> keys) {
+			EXPECT_CALL(damage_visitor_, visit(Eq(damage(level, keys, "foo")))).Times(1);
+		}
+
+		void run() {
+			// We must commit before we do the test to ensure
+			// all the block numbers and checksums are written
+			// to the btree nodes.
+			commit();
+
+			block_counter counter;
+			btree_damage_visitor<value_visitor_mock, damage_visitor_mock, 1, thing_traits>
+				visitor(counter, value_visitor_, damage_visitor_);
+			tree_->visit_depth_first(visitor);
+		}
+
 		with_temp_directory dir_;
 		block_manager<>::ptr bm_;
 		space_map::ptr sm_;
 		transaction_manager::ptr tm_;
 
 		thing_traits::ref_counter rc_;
-		btree<2, thing_traits>::ptr tree_;
+		btree<1, thing_traits>::ptr tree_;
 
 		value_visitor_mock value_visitor_;
 		damage_visitor_mock damage_visitor_;
@@ -82,35 +144,32 @@ namespace {
 
 //----------------------------------------------------------------
 
-TEST_F(BTreeDamageVisitorTests, visiting_an_empty_btree_visits_nothing)
+TEST_F(BTreeDamageVisitorTests, visiting_an_empty_tree)
 {
-	block_counter counter;
+	expect_no_values();
+	expect_no_damage();
 
-	EXPECT_CALL(value_visitor_, visit(_)).Times(0);
+	run();
-	EXPECT_CALL(damage_visitor_, visit(_)).Times(0);
-
-	btree_damage_visitor<value_visitor_mock, damage_visitor_mock, 2, thing_traits>
-		visitor(counter, value_visitor_, damage_visitor_);
-	tree_->visit_depth_first(visitor);
 }
 
 TEST_F(BTreeDamageVisitorTests, visiting_an_tree_with_a_trashed_root)
 {
-	zero_block(tree_->get_root());
+	trash_block(tree_->get_root());
-	EXPECT_CALL(value_visitor_, visit(_)).Times(0);
-	EXPECT_CALL(damage_visitor_, visit(Eq(damage(0, range<uint64_t>(), "foo")))).Times(1);
 
-	block_counter counter;
+	expect_no_values();
+	expect_damage(0, range<uint64_t>());
 
-	btree_damage_visitor<value_visitor_mock, damage_visitor_mock, 2, thing_traits>
+	run();
-		visitor(counter, value_visitor_, damage_visitor_);
-	tree_->visit_depth_first(visitor);
 }
 
-#if 0
+TEST_F(BTreeDamageVisitorTests, visiting_a_populated_tree_with_no_damage)
-	uint64_t key[2] = {1, 2};
+{
-	thing value = {0, 0};
+	insert_values(1000);
-	tree_->insert(key, value);
+
-#endif
+	expect_nr_values(1000);
+	expect_no_damage();
+
+	run();
+}
 
 //----------------------------------------------------------------