Cleanups in mapping_tree.cpp

[Christian Ehrhardt]

Bugfixes and cleanups in mapping_tree.cpp:
- The destructor should not stop after _count mappings as _count does
  not include unused mappings. This might cause leakage of mmemory wrt.
  quota.
- The skip_subtree variable in flush() is never true. Remove it and code
  that depends on it.
- Remove unused lookup() functions.
- The m_depth variable in flush() should have the depth value of the
  last mapping that was not unused or a submap. Currently, its use is
  inconsistent and this can AFAICS lead corrupt mapping trees, e.g. in
  this case:
       0: parent(depth = 0)
       1:   submap
       2:   child(depth = 1)
       3:    submap
  Assume a call with me_to == false and offsets that will
  not result in deletion of the submaps. Now look a the loop:
  Both the first and the second submap will be processed with
  m_depth=1 == p_depth+1, i.e. both submaps will potentially survive
  the flush. This is a bug as the tree now contains two consecutive
  submaps below parent.

        regards   Christian

diff --git a/src/kernel/fiasco/src/kern/mapping_tree.cpp b/src/kernel/fiasco/src/kern/mapping_tree.cpp
index d141d24..beee8a3 100644
--- a/src/kernel/fiasco/src/kern/mapping_tree.cpp
+++ b/src/kernel/fiasco/src/kern/mapping_tree.cpp
@@ -327,7 +327,7 @@ PUBLIC
 Mapping_tree::~Mapping_tree()
 {
   // special case for copied mapping trees
-  for (Mapping *m = _mappings; m < _mappings+_count && !m->is_end_tag(); ++m)
+  for (Mapping *m = _mappings; m < end() && !m->is_end_tag(); ++m)
     {
       if (!m->submap() && !m->unused())
         quota(m->space())->free(sizeof(Mapping));
@@ -730,8 +730,7 @@ Mapping_tree::flush(Mapping *parent, bool me_too,
   else
     start_of_deletions++;
 
-  unsigned m_depth = p_depth + 1;
-  bool skip_subtree = false;
+  unsigned m_depth = p_depth;
 
   for (Mapping* m = parent + 1;
        m < end() && ! m->is_end_tag();
@@ -755,18 +754,6 @@ Mapping_tree::flush(Mapping *parent, bool me_too,
           continue;
         }
 
-      // m is a submap or a regular mapping.
-      if (skip_subtree)
-        {
-          if (m->depth() > p_depth + 1) // Can use m->depth() even for submaps.
-            {
-              start_of_deletions++;
-              continue;
-            }
-
-          skip_subtree = false;
-        }
-
       Space *space;
 
       if (Treemap* submap = m->submap())
@@ -776,7 +763,7 @@ Mapping_tree::flush(Mapping *parent, bool me_too,
               // Check for immediate child.  Note: It's a bad idea to
               // call m->parent() because that would iterate backwards
               // over already-deleted entries.
-              && m_depth == p_depth + 1
+              && m_depth == p_depth
               && submap_ops.is_partial(submap, offs_begin, offs_end))
             {
               submap_ops.flush(submap, offs_begin, offs_end);
@@ -840,39 +827,6 @@ Mapping_tree::grant(Mapping* m, Space *new_space, Page_number va,
 
 PUBLIC inline
 Mapping *
-Mapping_tree::lookup(Space *space, Page_number page)
-{
-
-  Mapping *m;
-
-  for (m = mappings(); m; m = next(m))
-    {
-      assert (!m->submap());
-      if (m->space() == space && m->page() == page)
-        return m;
-    }
-
-  return 0;
-}
-
-
-PUBLIC
-Mapping *
-Base_mappable::lookup(Space *space, Page_number va)
-{
-  // get and lock the tree.
-  lock.lock();
-  Mapping_tree *t = tree.get();
-  assert (t);
-  if (Mapping *m = t->lookup(space, va))
-    return m;
-
-  lock.clear();
-  return 0;
-}
-
-PUBLIC inline
-Mapping *
 Base_mappable::insert(Mapping* parent, Space *space, Page_number page)
 {
   Mapping_tree* t = tree.get();