From b4e1f173dbde483fff58d6454b11c421d9201230 Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Thu, 9 May 2013 15:15:40 +0100 Subject: [PATCH] Changing required keyword arguments to cause an assertion rather than return a status code, and removing repetition in how this is expressed. Copied from Perforce Change: 181674 ServerID: perforce.ravenbrook.com --- mps/code/arenacl.c | 20 ++++++-------------- mps/code/arenavm.c | 8 ++------ mps/code/arg.c | 9 +++++++++ mps/code/arg.h | 2 +- mps/code/dbgpool.c | 10 +++------- mps/code/poolamc.c | 4 ++-- mps/code/poolams.c | 27 ++++++--------------------- mps/code/poolawl.c | 18 +++++------------- mps/code/poollo.c | 10 +++------- mps/code/poolmfs.c | 16 ++++------------ mps/code/poolmv2.c | 37 ++++++++++++------------------------- mps/code/poolsnc.c | 12 ++---------- mps/code/segsmss.c | 12 ++---------- 13 files changed, 57 insertions(+), 128 deletions(-) diff --git a/mps/code/arenacl.c b/mps/code/arenacl.c index db10accd6fe..d0e65a53ac1 100644 --- a/mps/code/arenacl.c +++ b/mps/code/arenacl.c @@ -216,19 +216,11 @@ static Res ClientArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args) AVER(arenaReturn != NULL); AVER((ArenaClass)mps_arena_class_cl() == class); AVER(ArgListCheck(args)); - - if (ArgPick(&arg, args, MPS_KEY_ARENA_SIZE)) { - size = arg.val.size; - if (ArgPick(&arg, args, MPS_KEY_ARENA_CL_ADDR)) - base = arg.val.addr; - else { - res = ResPARAM; - goto failParam; - } - } else { - res = ResPARAM; - goto failParam; - } + + ArgRequire(&arg, args, MPS_KEY_ARENA_SIZE); + size = arg.val.size; + ArgRequire(&arg, args, MPS_KEY_ARENA_CL_ADDR); + base = arg.val.addr; AVER(base != (Addr)0); @@ -272,7 +264,7 @@ static Res ClientArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args) failChunkCreate: ArenaFinish(arena); -failParam: + AVER(res != ResOK); return res; } diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c index 0bb83500f25..9904ada470a 100644 --- a/mps/code/arenavm.c +++ b/mps/code/arenavm.c @@ -478,12 +478,8 @@ static Res VMArenaInit(Arena *arenaReturn, ArenaClass class, ArgList args) AVER(class == VMArenaClassGet() || class == VMNZArenaClassGet()); AVER(ArgListCheck(args)); - if (ArgPick(&arg, args, MPS_KEY_ARENA_SIZE)) - userSize = arg.val.size; - else { - res = ResPARAM; - goto failVMCreate; - } + ArgRequire(&arg, args, MPS_KEY_ARENA_SIZE); + userSize = arg.val.size; AVER(userSize > 0); diff --git a/mps/code/arg.c b/mps/code/arg.c index 4d2447e5c00..14170c3ed35 100644 --- a/mps/code/arg.c +++ b/mps/code/arg.c @@ -142,6 +142,15 @@ found: } +/* ArgRequire -- take a required argument out of the argument list by keyword */ + +void ArgRequire(ArgStruct *argOut, ArgList args, Key key) { + if (ArgPick(argOut, args, key)) + return; + NOTREACHED; +} + + /* C. COPYRIGHT AND LICENSE * * Copyright (C) 2001-2013 Ravenbrook Limited . diff --git a/mps/code/arg.h b/mps/code/arg.h index 2647937c201..dc1e8a2a991 100644 --- a/mps/code/arg.h +++ b/mps/code/arg.h @@ -37,7 +37,7 @@ extern Bool ArgCheck(Arg arg); extern Bool ArgListCheck(ArgList args); extern Bool ArgPick(ArgStruct *argOut, ArgList args, Key key); -extern void ArgRequired(ArgStruct *argOut, ArgList args, Key key); +extern void ArgRequire(ArgStruct *argOut, ArgList args, Key key); extern Bool ArgCheckCant(Arg arg); extern Bool ArgCheckFormat(Arg arg); diff --git a/mps/code/dbgpool.c b/mps/code/dbgpool.c index d70e855adc3..88dec0532b6 100644 --- a/mps/code/dbgpool.c +++ b/mps/code/dbgpool.c @@ -132,12 +132,8 @@ static Res DebugPoolInit(Pool pool, ArgList args) /* TODO: Split this structure into separate keyword arguments, now that we can support them. */ - if (ArgPick(&arg, args, MPS_KEY_POOL_DEBUG_OPTIONS)) - options = (PoolDebugOptions)arg.val.pool_debug_options; - else { - res = ResPARAM; - goto failParam; - } + ArgRequire(&arg, args, MPS_KEY_POOL_DEBUG_OPTIONS); + options = (PoolDebugOptions)arg.val.pool_debug_options; AVERT(PoolDebugOptions, options); @@ -208,7 +204,7 @@ static Res DebugPoolInit(Pool pool, ArgList args) tagFail: alignFail: SuperclassOfPool(pool)->finish(pool); -failParam: + AVER(res != ResOK); return res; } diff --git a/mps/code/poolamc.c b/mps/code/poolamc.c index 8235b66c05f..b6b4e5c0289 100644 --- a/mps/code/poolamc.c +++ b/mps/code/poolamc.c @@ -973,9 +973,9 @@ static Res amcInitComm(Pool pool, RankSet rankSet, ArgList args) amc = Pool2AMC(pool); arena = PoolArena(pool); - ArgRequired(&arg, args, MPS_KEY_FORMAT); + ArgRequire(&arg, args, MPS_KEY_FORMAT); pool->format = arg.val.format; - ArgRequired(&arg, args, MPS_KEY_CHAIN); + ArgRequire(&arg, args, MPS_KEY_CHAIN); amc->chain = arg.val.chain; AVERT(Format, pool->format); diff --git a/mps/code/poolams.c b/mps/code/poolams.c index 6d4b8615abf..e5e4b249bcb 100644 --- a/mps/code/poolams.c +++ b/mps/code/poolams.c @@ -769,24 +769,12 @@ static Res AMSInit(Pool pool, ArgList args) AVERT(Pool, pool); AVER(ArgListCheck(args)); - if (ArgPick(&arg, args, MPS_KEY_CHAIN)) - chain = arg.val.chain; - else { - res = ResPARAM; - goto failParam; - } - if (ArgPick(&arg, args, MPS_KEY_FORMAT)) - format = arg.val.format; - else { - res = ResPARAM; - goto failParam; - } - if (ArgPick(&arg, args, MPS_KEY_AMS_SUPPORT_AMBIGUOUS)) - supportAmbiguous = arg.val.b; - else { - res = ResPARAM; - goto failParam; - } + ArgRequire(&arg, args, MPS_KEY_CHAIN); + chain = arg.val.chain; + ArgRequire(&arg, args, MPS_KEY_FORMAT); + format = arg.val.format; + ArgRequire(&arg, args, MPS_KEY_AMS_SUPPORT_AMBIGUOUS); + supportAmbiguous = arg.val.b; /* .ambiguous.noshare: If the pool is required to support ambiguous */ /* references, the alloc and white tables cannot be shared. */ @@ -795,9 +783,6 @@ static Res AMSInit(Pool pool, ArgList args) EVENT3(PoolInitAMS, pool, PoolArena(pool), format); } return res; - -failParam: - return res; } diff --git a/mps/code/poolawl.c b/mps/code/poolawl.c index 95534d66958..96f0d2bef03 100644 --- a/mps/code/poolawl.c +++ b/mps/code/poolawl.c @@ -541,18 +541,10 @@ static Res AWLInit(Pool pool, ArgList args) awl = Pool2AWL(pool); - if (ArgPick(&arg, args, MPS_KEY_FORMAT)) - format = arg.val.format; - else { - res = ResPARAM; - goto failParam; - } - if (ArgPick(&arg, args, MPS_KEY_AWL_FIND_DEPENDENT)) - findDependent = (FindDependentMethod)arg.val.addr_method; - else { - res = ResPARAM; - goto failParam; - } + ArgRequire(&arg, args, MPS_KEY_FORMAT); + format = arg.val.format; + ArgRequire(&arg, args, MPS_KEY_AWL_FIND_DEPENDENT); + findDependent = (FindDependentMethod)arg.val.addr_method; AVERT(Format, format); pool->format = format; @@ -584,7 +576,7 @@ static Res AWLInit(Pool pool, ArgList args) failGenInit: ChainDestroy(chain); -failParam: + AVER(res != ResOK); return res; } diff --git a/mps/code/poollo.c b/mps/code/poollo.c index 541f0901781..8943f37c8ba 100644 --- a/mps/code/poollo.c +++ b/mps/code/poollo.c @@ -493,12 +493,8 @@ static Res LOInit(Pool pool, ArgList args) arena = PoolArena(pool); - if (ArgPick(&arg, args, MPS_KEY_FORMAT)) - format = arg.val.format; - else { - res = ResPARAM; - goto failParam; - } + ArgRequire(&arg, args, MPS_KEY_FORMAT); + format = arg.val.format; AVERT(Format, format); @@ -525,7 +521,7 @@ static Res LOInit(Pool pool, ArgList args) failGenInit: ChainDestroy(lo->chain); -failParam: + AVER(res != ResOK); return res; } diff --git a/mps/code/poolmfs.c b/mps/code/poolmfs.c index 4a33ed050a1..538be571934 100644 --- a/mps/code/poolmfs.c +++ b/mps/code/poolmfs.c @@ -93,8 +93,8 @@ ARG_DEFINE_KEY(mfs_unit_size, Size); static Res MFSInit(Pool pool, ArgList args) { - Res res; - Size extendBy, unitSize; + Size extendBy = MFS_EXTEND_BY_DEFAULT; + Size unitSize; MFS mfs; Arena arena; ArgStruct arg; @@ -102,16 +102,11 @@ static Res MFSInit(Pool pool, ArgList args) AVER(pool != NULL); AVER(ArgListCheck(args)); - if (ArgPick(&arg, args, MPS_KEY_MFS_UNIT_SIZE)) - unitSize = arg.val.size; - else { - res = ResPARAM; - goto failParam; - } + ArgRequire(&arg, args, MPS_KEY_MFS_UNIT_SIZE); + unitSize = arg.val.size; if (ArgPick(&arg, args, MPS_KEY_EXTEND_BY)) extendBy = arg.val.size; else { - extendBy = MFS_EXTEND_BY_DEFAULT; if (extendBy < unitSize) extendBy = unitSize; } @@ -137,9 +132,6 @@ static Res MFSInit(Pool pool, ArgList args) AVERT(MFS, mfs); EVENT4(PoolInitMFS, pool, arena, extendBy, unitSize); return ResOK; - -failParam: - return res; } diff --git a/mps/code/poolmv2.c b/mps/code/poolmv2.c index 203bf5a8e36..076d39563b4 100644 --- a/mps/code/poolmv2.c +++ b/mps/code/poolmv2.c @@ -242,36 +242,23 @@ static Res MVTInit(Pool pool, ArgList args) arena = PoolArena(pool); AVERT(Arena, arena); - - /* FIXME: Inconsistent reporting of bad arguments. Elsewhere we assert or return ResPARAM. */ - /* --- Should there be a ResBADARG ? */ - if (ArgPick(&arg, args, MPS_KEY_MIN_SIZE)) { + if (ArgPick(&arg, args, MPS_KEY_MIN_SIZE)) minSize = arg.val.size; - unless (minSize > 0) - return ResLIMIT; - } - if (ArgPick(&arg, args, MPS_KEY_MEAN_SIZE)) { + if (ArgPick(&arg, args, MPS_KEY_MEAN_SIZE)) meanSize = arg.val.size; - unless (meanSize >= minSize) - return ResLIMIT; - } - if (ArgPick(&arg, args, MPS_KEY_MAX_SIZE)) { + if (ArgPick(&arg, args, MPS_KEY_MAX_SIZE)) maxSize = arg.val.size; - unless (maxSize >= meanSize) - return ResLIMIT; - } - if (ArgPick(&arg, args, MPS_KEY_MVT_RESERVE_DEPTH)) { - /* --- check that maxSize is not too large */ + if (ArgPick(&arg, args, MPS_KEY_MVT_RESERVE_DEPTH)) reserveDepth = arg.val.count; - unless (reserveDepth > 0) - return ResLIMIT; - } - if (ArgPick(&arg, args, MPS_KEY_MVT_FRAG_LIMIT)) { - /* --- check that reserveDepth is not too large or small */ + if (ArgPick(&arg, args, MPS_KEY_MVT_FRAG_LIMIT)) fragLimit = arg.val.count; - unless (fragLimit <= 100) - return ResLIMIT; - } + + AVER(0 < minSize); + AVER(minSize <= meanSize); + AVER(meanSize <= maxSize); + AVER(reserveDepth > 0); + AVER(fragLimit <= 100); + /* TODO: More sanity checks possible? */ /* see */ fillSize = SizeAlignUp(maxSize, ArenaAlign(arena)); diff --git a/mps/code/poolsnc.c b/mps/code/poolsnc.c index 6b01c3a9b02..70c618bf385 100644 --- a/mps/code/poolsnc.c +++ b/mps/code/poolsnc.c @@ -363,7 +363,6 @@ static Bool sncFindFreeSeg(Seg *segReturn, SNC snc, Size size) static Res SNCInit(Pool pool, ArgList args) { - Res res; SNC snc; Format format; ArgStruct arg; @@ -373,12 +372,8 @@ static Res SNCInit(Pool pool, ArgList args) snc = Pool2SNC(pool); - if (ArgPick(&arg, args, MPS_KEY_FORMAT)) - format = arg.val.format; - else { - res = ResPARAM; - goto failParam; - } + ArgRequire(&arg, args, MPS_KEY_FORMAT); + format = arg.val.format; AVERT(Format, format); pool->format = format; @@ -391,9 +386,6 @@ static Res SNCInit(Pool pool, ArgList args) AVERT(SNC, snc); EVENT2(PoolInitSNC, pool, format); return ResOK; - -failParam: - return res; } diff --git a/mps/code/segsmss.c b/mps/code/segsmss.c index 9d69aff302a..02b29504d34 100644 --- a/mps/code/segsmss.c +++ b/mps/code/segsmss.c @@ -341,12 +341,8 @@ static Res AMSTInit(Pool pool, ArgList args) AVERT(Pool, pool); - if (ArgPick(&arg, args, MPS_KEY_FORMAT)) - format = arg.val.format; - else { - res = ResPARAM; - goto failParam; - } + ArgRequire(&arg, args, MPS_KEY_FORMAT); + format = arg.val.format; res = ChainCreate(&chain, pool->arena, 1, &genParam); if (res != ResOK) @@ -369,10 +365,6 @@ static Res AMSTInit(Pool pool, ArgList args) amst->sig = AMSTSig; AVERT(AMST, amst); return ResOK; - -failParam: - AVER(res != ResOK); - return res; }