From ae5a06f21bacefec79a77292d1fc64c0509ea2a6 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 29 May 2013 12:19:19 +0100 Subject: [PATCH] Fix review comments from : * Rename RangeOverlap to RangesOverlap. * MVFF shouldn't assume that CBSInsert and CBSDelete can't fail. * Remove unused function mps_mvff_stat. * Don't call range methods in RangeCheck. * RangeInit can't fail, so return void. Copied from Perforce Change: 182297 ServerID: perforce.ravenbrook.com --- mps/code/poolmv2.c | 2 +- mps/code/poolmvff.c | 39 +++++++++++++-------------------------- mps/code/range.c | 10 ++++------ mps/code/range.h | 4 ++-- mps/design/range.txt | 4 ++-- 5 files changed, 22 insertions(+), 37 deletions(-) diff --git a/mps/code/poolmv2.c b/mps/code/poolmv2.c index f49880023b9..ac0e4e87b50 100644 --- a/mps/code/poolmv2.c +++ b/mps/code/poolmv2.c @@ -605,7 +605,7 @@ static Bool MVTDeleteOverlapping(ABQDisposition *dispositionReturn, oldRange = element; newRange = closureP; - if (RangeOverlap(oldRange, newRange)) { + if (RangesOverlap(oldRange, newRange)) { *dispositionReturn = ABQDispositionDELETE; } else { *dispositionReturn = ABQDispositionKEEP; diff --git a/mps/code/poolmvff.c b/mps/code/poolmvff.c index f557a4e4da7..95e09d90b26 100644 --- a/mps/code/poolmvff.c +++ b/mps/code/poolmvff.c @@ -80,7 +80,7 @@ typedef MVFFDebugStruct *MVFFDebug; * * Updates MVFF counters for additional free space. Returns maximally * coalesced range containing given range. Does not attempt to free - * segments (see MVFFFreeSegs). Cannot(!) fail. + * segments (see MVFFFreeSegs). */ static void MVFFAddToFreeList(Addr *baseIO, Addr *limitIO, MVFF mvff) { Res res; @@ -94,6 +94,10 @@ static void MVFFAddToFreeList(Addr *baseIO, Addr *limitIO, MVFF mvff) { AVER(limit > base); res = CBSInsert(baseIO, limitIO, CBSOfMVFF(mvff), base, limit); + if (ResIsAllocFailure(res)) + /* CBS ran out of memory for splay nodes: lose freed range. */ + return; + AVER(res == ResOK); mvff->free += AddrOffset(base, limit); @@ -138,6 +142,10 @@ static void MVFFFreeSegs(MVFF mvff, Addr base, Addr limit) if (segBase >= base) { /* segment starts in range */ Addr oldBase, oldLimit; res = CBSDelete(&oldBase, &oldLimit, CBSOfMVFF(mvff), segBase, segLimit); + if (ResIsAllocFailure(res)) + /* CBS ran out of memory for splay nodes: can't delete. */ + return; + AVER(res == ResOK); AVER(oldBase <= segBase); AVER(segLimit <= oldLimit); @@ -367,6 +375,10 @@ static Res MVFFBufferFill(Addr *baseReturn, Addr *limitReturn, Addr newBase, newLimit; foundBlock = FALSE; res = CBSInsert(&newBase, &newLimit, CBSOfMVFF(mvff), base, limit); + if (ResIsAllocFailure(res)) + /* CBS ran out of memory for splay nodes: lose block. */ + return res; + AVER(newBase == base); AVER(newLimit == limit); AVER(res == ResOK); @@ -722,31 +734,6 @@ static Bool MVFFCheck(MVFF mvff) } -/* mps_mvff_stat -- a hack to get statistics emitted - * - * .stat: The SW temp pool cannot be destroyed, so we're providing this - * to get the statistics. It breaks modularity to access CBS internals. - */ - -#include "meter.h" -extern void mps_mvff_stat(mps_pool_t pool); - -void mps_mvff_stat(mps_pool_t mps_pool) -{ - Pool pool; - MVFF mvff; - - pool = (Pool)mps_pool; - AVERT(Pool, pool); - mvff = Pool2MVFF(pool); - AVERT(MVFF, mvff); - - METER_EMIT(&CBSOfMVFF(mvff)->splaySearch); - /* FIXME: METER_EMIT(&CBSOfMVFF(mvff)->eblSearch); */ - /* FIXME: METER_EMIT(&CBSOfMVFF(mvff)->eglSearch); */ -} - - /* C. COPYRIGHT AND LICENSE * * Copyright (C) 2001-2002 Ravenbrook Limited . diff --git a/mps/code/range.c b/mps/code/range.c index 739b11f5b5e..60a8b0da367 100644 --- a/mps/code/range.c +++ b/mps/code/range.c @@ -16,14 +16,13 @@ SRCID(range, "$Id$"); Bool RangeCheck(Range range) { CHECKS(Range, range); - CHECKL(range != NULL); - CHECKL(RangeBase(range) != NULL); - CHECKL(RangeBase(range) <= RangeLimit(range)); + CHECKL(range->base != NULL); + CHECKL(range->base <= range->limit); return TRUE; } -Res RangeInit(Range range, Addr base, Addr limit) +void RangeInit(Range range, Addr base, Addr limit) { AVER(range != NULL); AVER(base != NULL); @@ -34,7 +33,6 @@ Res RangeInit(Range range, Addr base, Addr limit) range->sig = RangeSig; AVERT(Range, range); - return ResOK; } void RangeFinish(Range range) @@ -64,7 +62,7 @@ Res RangeDescribe(Range range, mps_lib_FILE *stream) return ResOK; } -Bool RangeOverlap(Range range1, Range range2) +Bool RangesOverlap(Range range1, Range range2) { AVERT(Range, range1); AVERT(Range, range2); diff --git a/mps/code/range.h b/mps/code/range.h index 298cb01ff89..c6c70ddb1c4 100644 --- a/mps/code/range.h +++ b/mps/code/range.h @@ -27,12 +27,12 @@ typedef struct RangeStruct *Range; #define RangeLimit(range) ((range)->limit) #define RangeSize(range) (AddrOffset(RangeBase(range), RangeLimit(range))) -extern Res RangeInit(Range range, Addr base, Addr limit); +extern void RangeInit(Range range, Addr base, Addr limit); extern void RangeFinish(Range range); extern Res RangeDescribe(Range range, mps_lib_FILE *stream); extern Bool RangeCheck(Range range); extern Bool RangeIsAligned(Range range, Align align); -extern Bool RangeOverlap(Range range1, Range range2); +extern Bool RangesOverlap(Range range1, Range range2); extern Addr (RangeBase)(Range range); extern Addr (RangeLimit)(Range range); extern Size (RangeSize)(Range range); diff --git a/mps/design/range.txt b/mps/design/range.txt index 8a97d7db3f5..685809c54bf 100644 --- a/mps/design/range.txt +++ b/mps/design/range.txt @@ -43,7 +43,7 @@ Interface inlined in client structures or allocated on the stack. Clients must not depend on its implementation details. -``Res RangeInit(Range range, Addr base, Addr limit)`` +``void RangeInit(Range range, Addr base, Addr limit)`` Initialize a range object to represent the half-open address range between ``base`` (inclusive) and ``limit`` (exclusive). It must be the @@ -72,7 +72,7 @@ there is a function too.) Return the size of the range. (This is implemented as a macro, but there is a function too. The macro evaluates its argument twice.) -``Bool RangeOverlap(Range range1, Range range2)`` +``Bool RangesOverlap(Range range1, Range range2)`` Return ``TRUE`` if the two ranges overlap (have at least one address in common), or ``FALSE`` if they do not. Note that ranges [*A*, *B*) and