Fix issues identified in review by gdr <https://info.ravenbrook.com/mail/2018/07/05/10-17-35/0/>

Copied from Perforce
 Change: 194430

mps/code/cbs.c

@@ -6,8 +6,8 @@
  * .intro: This is a portable implementation of coalescing block
  * structures.
  *
- * .purpose: CBSs are used to manage potentially unbounded
- * collections of memory blocks.
+ * .purpose: CBSs are used to manage potentially unbounded collections
+ * of memory blocks.
  *
  * .sources: <design/cbs/>.
  */
@@ -25,13 +25,13 @@ SRCID(cbs, "$Id$");
 
 #define cbsOfLand(land) PARENT(CBSStruct, landStruct, land)
 #define cbsSplay(cbs) (&((cbs)->splayTreeStruct))
-#define cbsOfSplay(_splay) PARENT(CBSStruct, splayTreeStruct, _splay)
-#define cbsFastBlockOfTree(_tree) \
-  PARENT(CBSFastBlockStruct, nodeStruct, NodeOfTree(_tree))
-#define cbsFastBlockNode(_block) (&(_block)->nodeStruct)
-#define cbsZonedBlockOfTree(_tree) \
-  PARENT(CBSZonedBlockStruct, cbsFastBlockStruct, cbsFastBlockOfTree(_tree))
-#define cbsZonedBlockNode(_block) cbsFastBlockNode(&(_block)->cbsFastBlockStruct)
+#define cbsOfSplay(splay) PARENT(CBSStruct, splayTreeStruct, splay)
+#define cbsFastBlockOfTree(tree) \
+  PARENT(CBSFastBlockStruct, nodeStruct, NodeOfTree(tree))
+#define cbsFastBlockNode(block) (&(block)->nodeStruct)
+#define cbsZonedBlockOfTree(tree) \
+  PARENT(CBSZonedBlockStruct, cbsFastBlockStruct, cbsFastBlockOfTree(tree))
+#define cbsZonedBlockNode(block) cbsFastBlockNode(&(block)->cbsFastBlockStruct)
 #define cbsBlockPool(cbs) RVALUE((cbs)->blockPool)
 
 
@@ -405,22 +405,22 @@ static Res cbsInsert(Range rangeReturn, Land land, Range range)
   limit = RangeLimit(range);
 
   METER_ACC(cbs->treeSearch, cbs->treeSize);
-  b = SplayTreeNeighbours(&leftSplay, &rightSplay,
-                          cbsSplay(cbs), NodeKeyOfBaseVar(base));
+  b = SplayTreeNeighbours(&leftSplay, &rightSplay, cbsSplay(cbs),
+                          NodeKeyOfBaseVar(base));
   if (!b) {
     res = ResFAIL;
     goto fail;
   }
 
   /* .insert.overlap: The two cases below are not quite symmetrical,
-     because base was passed into the call to SplayTreeNeighbours(),
-     but limit was not.  So we know that if there is a left neighbour,
-     then leftBlock's limit <= base (this is ensured by NodeCompare,
-     which is the comparison method on the tree).  But if there is a
-     right neighbour, all we know is that base < rightBlock's
-     base. But for the range to fit, we need limit <= rightBlock's
-     base too. Hence the extra check and the possibility of failure in
-     the second case. */
+     because base was passed into the call to SplayTreeNeighbours(), but
+     limit was not. So we know that if there is a left neighbour, then
+     NodeLimit(leftBlock) <= base (this is ensured by NodeCompare, which
+     is the comparison method on the tree). But if there is a right
+     neighbour, all we know is that base < NodeBase(rightBlock). But for
+     the range to fit, we need limit <= NodeBase(rightBlock) too. Hence
+     the extra check and the possibility of failure in the second
+     case. */
 
   if (leftSplay == TreeEMPTY) {
     leftBlock = NULL;
@@ -436,7 +436,7 @@ static Res cbsInsert(Range rangeReturn, Land land, Range range)
     rightMerge = FALSE;
   } else {
     rightBlock = NodeOfTree(rightSplay);
-    if (rightBlock != NULL && limit > NodeLimit(rightBlock)) {
+    if (rightBlock != NULL && limit > NodeBase(rightBlock)) {
       /* .insert.overlap */
       res = ResFAIL;
       goto fail;
@@ -861,7 +861,7 @@ static Bool cbsFindFirst(Range rangeReturn, Range oldRangeReturn,
 
 /* cbsFindInZones -- find a block within a zone set
  *
- * Fins a block of at least the given size that lies entirely within a
+ * Finds a block of at least the given size that lies entirely within a
  * zone set. (The first such block, if high is FALSE, or the last, if
  * high is TRUE.)
  */
@@ -882,6 +882,7 @@ static Bool cbsTestNodeInZones(SplayTree splay, Tree tree,
   cbsTestNodeInZonesClosure my = closure;
   RangeInZoneSet search;
 
+  AVER_CRITICAL(closure != NULL);
   UNUSED(splay);
 
   search = my->high ? RangeInZoneSetLast : RangeInZoneSetFirst;
@@ -898,6 +899,7 @@ static Bool cbsTestTreeInZones(SplayTree splay, Tree tree,
   CBSZonedBlock zonedBlock = cbsZonedBlockOfTree(tree);
   cbsTestNodeInZonesClosure my = closure;
   
+  AVER_CRITICAL(closure != NULL);
   UNUSED(splay);
   
   return fastBlock->maxSize >= my->size

mps/code/cbs.h

@@ -12,6 +12,7 @@
 #include "arg.h"
 #include "mpmtypes.h"
 #include "mpmst.h"
+#include "node.h"
 #include "range.h"
 #include "splay.h"
 

mps/code/mpmst.h

@@ -240,28 +240,6 @@ typedef struct SegClassStruct {
 } SegClassStruct;
 
 
-/* RangeStruct -- address range
- *
- * See design.mps.range, range.h, range.c.
- */
-
-typedef struct RangeStruct {
-  Addr base;
-  Addr limit;
-} RangeStruct;
-
-
-/* NodeStruct -- address range in a tree
- *
- * See node.h, node.c.
- */
-
-typedef struct NodeStruct {
-  TreeStruct treeStruct;
-  RangeStruct rangeStruct;
-} NodeStruct;
-
-
 /* SegStruct -- segment structure
  *
  * .seg: Segments are the basic units of protection and tracer activity

mps/code/node.h

@@ -8,6 +8,7 @@
 #define node_h
 
 #include "mpmtypes.h"
+#include "range.h"
 #include "tree.h"
 
 #define NodeTree(node) (&(node)->treeStruct)
@@ -29,10 +30,10 @@ extern void NodeFinish(Node node);
 
 /* Functions for nodes in trees
  *
- * We pass the ndoe base directly as a TreeKey (void *) assuming that
+ * We pass the node base directly as a TreeKey (void *) assuming that
  * Addr can be encoded, and possibly breaking <design/type/#addr.use>.
  * On an exotic platform where this isn't true, pass the address of
- * base.  i.e. add an &
+ * base: that is, add an &.
  */
 
 #define NodeKeyOfBaseVar(baseVar) ((TreeKey)(baseVar))
@@ -42,6 +43,13 @@ extern Compare NodeCompare(Tree tree, TreeKey key);
 extern TreeKey NodeKey(Tree tree);
 
 
+/* NodeStruct -- address range in a tree */
+
+typedef struct NodeStruct {
+  TreeStruct treeStruct;
+  RangeStruct rangeStruct;
+} NodeStruct;
+
 #endif /* node_h */
 
 /* C. COPYRIGHT AND LICENSE

mps/code/range.c

@@ -41,7 +41,7 @@ void RangeFinish(Range range)
   AVERT(Range, range);
   /* Make range invalid and recognisably so. */
   range->limit = (Addr)0;
-  range->base = (Addr)0xF191583D;
+  range->base = (Addr)0xF191583D; /* FINISHED */
 }
 
 Res RangeDescribe(Range range, mps_lib_FILE *stream, Count depth)

mps/code/range.h

@@ -40,6 +40,12 @@ extern void (RangeSetLimit)(Range range, Addr addr);
 extern Size (RangeSize)(Range range);
 extern void RangeCopy(Range to, Range from);
 
+/* RangeStruct -- address range */
+
+typedef struct RangeStruct {
+  Addr base;
+  Addr limit;
+} RangeStruct;
 
 #endif /* range_h */
 

mps/design/cbs.txt

@@ -111,7 +111,9 @@ following optional keyword arguments:
   the block descriptor pool automatically extends itself when out of
   space; if ``FALSE``, the pool returns ``ResLIMIT`` in this case.
   (This feature is used by the arena to bootstrap its own CBS of free
-  memory. See design.mps.bootstrap.land.sol.pool.)
+  memory. See design.mps.bootstrap.land.sol.pool_.)
+
+  .. _design.mps.bootstrap.land.sol.pool: bootstrap#land-sol-pool
 
 
 Limitations
@@ -238,10 +240,10 @@ this would make coalescence slightly less eager, by up to
 
 _`.future.iterate.and.delete`: It would be possible to provide an
 implementation for the ``LandIterateAndDelete()`` generic function
-using ``TreeTraverseAndDelete``, which calls ``TreeToVine()`` first,
+using ``TreeTraverseAndDelete()``, which calls ``TreeToVine()`` first,
 iterates over the vine (where deletion is straightforward), and then
-rebalances the tree.  Note that this is little better than using
-``SplayFirst`` and ``SplayNext``.
+rebalances the tree. Note that this is little better than using
+``SplayFirst()`` and ``SplayNext()``.
 
 _`.future.lazy-coalesce`: It's long been observed that small blocks
 are often freed and then reallocated, so that coalescing them is a
@@ -299,7 +301,7 @@ Document History
   Documented new keyword arguments.
 
 - 2016-03-27 RB_ Adding cross references to usage. Updating future
-  with reference to ``TreeTraverseAndDelete``. Adding future idea
+  with reference to ``TreeTraverseAndDelete()``. Adding future idea
   about lazy coalescing.
   
 .. _RB: http://www.ravenbrook.com/consultants/rb/

mps/design/range.txt

@@ -81,14 +81,14 @@ there is a function too.)
 
 ``void RangeSetBase(Range range, Addr addr)``
 
-Set the base of the range.  ``addr`` must not be greater than the
-range limit.  To set them both at once, use ``RangeInit``.  (This is
+Set the base of the range. ``addr`` must not be greater than the range
+limit. To set them both at once, use ``RangeInit()``. (This is
 implemented as a macro, but there is a function too.)
 
 ``void RangeSetLimit(Range range, Addr addr)``
 
-Set the limit of the range.  ``addr`` must not be less than the range
-base.  To set the both at once, use ``RangeInit``.  (This is
+Set the limit of the range. ``addr`` must not be less than the range
+base. To set the both at once, use ``RangeInit()``. (This is
 implemented as a macro, but there's a function too.)
 
 ``Size RangeSize(Range range)``
@@ -129,8 +129,8 @@ Document history
 ----------------
 
 - 2013-05-21 GDR_ Created.
-- 2014-01-15 GDR_ Added ``RangeContains``.
-- 2016-03-27 RB_ Addded ``RangeSetBase`` and ``RangeSetLimit``.
+- 2014-01-15 GDR_ Added ``RangeContains()``.
+- 2016-03-27 RB_ Addded ``RangeSetBase()`` and ``RangeSetLimit()``.
 
 .. _GDR: http://www.ravenbrook.com/consultants/gdr/
 .. _RB: http://www.ravenbrook.com/consultants/rb/