Ravenbrook / Projects / Memory Pool System / Issues


MPS issue job000742

Title:Some niggles annoy DRJ
Status:open
Priority:nice
Assigned user:Gareth Rees
Organization:Ravenbrook
Description:There's a growing pile of things that annoy DRJ. Mostly slightly untidy code and practices. Let's have a list.

- C style: inconsistent use of space in '++ variable'. Although I usually use '++variable' I quite like '++ variable' as it makes it stand out more, and that's fine for an idiom.
- SegSetSummary in seg.c has a very naughty #ifdef PROTECTION_NONE, as does SegSetRankAndSummary. grep for PROTECTION_NONE and #ifdef?
- seg.c has names beginning with 'GC'
- ArenaRootRing macro in mpm.h is not used (and can not be). ArenaTraceRing macro is not used. How do we find other broken unused macros?
- Ghastly use of ternary inside "if" in amcReclaimNailed.
Analysis:GDR 2013-03-08: It's not appropriate to fix the following:

- what's with all the static declarations? They impede debugging. EG the backtrace that Göran mailed round[1] didn't have the right symbol for seg.c line 751 because the function was declared static. [GDR 2013-03-08: the "static" declaration allows compilers like LLVM to aggressively inline functions.]

GDR 2012-10-26: The following items are be fixed:

- C style: inconsistent use of space after 'if' 'for' 'while' etc. We should document one style and stick to it. I prefer "if(...)" not "if (...)" [DRJ 2007-07-11: this is now documented in design/cstyle]

- fri4gc.gmk why is it i4? [GDR 2012-10-26: mistake; now fri3gc]

- fri4gc.gmk -Wpointer-arith is suppressed, but this appears to be for SuSE, is this necessary? [GDR 2012-10-26: not there any more]

- mpsi.c mps_space_create and mps_space_destroy are #ifdef MPS_PROD_DYLAN. We don't support dylan or these functions. remove them? [GDR 2012-10-26: last mention of mps_space removed from mpsi.c in change 180093.]

- lines with multiple statements in. Such as '*segReturn = seg; *rankReturn = rank' in trace.c [DRJ 2007-07-11: design/cstyle now bans this] search for ';.*;' ?

- mps_arena_park in mpsi.c takes mps_space_t, not mps_arena_t [GDR 2012-10-26: not any more]

- mps_arena_has_addr in reference manual has all sorts of silly references to mps_arena_park. [GDR 2012-10-26: not any more]

- pol should be pool in test_stepper in amcss.c, root conflicts with global. rename global.
How found:inspection
Evidence:Looking at the code mostly
[1] /​/​info.ravenbrook.com/​mail/​2003/​11/​17/​15-10-02/​0.txt
Created by:David Jones
Created on:2003‑07‑09 11:43:02
Last modified by:Gareth Rees
Last modified on:2013‑03‑19 12:07:34
History:2003-07-09 DRJ created
2003-11-04 DRJ PROTECTION_NONE #ifdef
2003-11-19 DRJ too much static
2007-07-11 DRJ ternary
2012-10-26 GDR some of this has been fixed
2013-03-19 GDR Assigned to GDR.

Fixes

Change Effect Date User Description
181095 open 2013‑03‑08 12:41:41 Gareth Rees Move global 'pool' to local so that we can rename argument 'pol' to 'pool'.
180093 open 2012‑10‑26 09:51:12 Gareth Rees No need to include "mpsavm.h". The comment justifying it was bogus: "only for mps_space_create".
178941 open 2012‑08‑15 17:21:18 Richard Brooksby Removing no-longer-needed warning suppression.
178940 open 2012‑08‑15 17:04:55 Richard Brooksby Removing no-longer-required warning suppression.
178939 open 2012‑08‑15 17:01:48 Richard Brooksby Removing no-longer-required warning supression.
178938 open 2012‑08‑15 17:00:58 Richard Brooksby Removing no-longer-needed warning suppression.
178935 open 2012‑08‑15 15:37:42 Richard Brooksby Correcting misnamed I4 architecture to I3. This distinction was once slightly useful for optimisation, but no longer makes any sense.

Generated at 2013-05-24 08:37:40 by $Id: //info.ravenbrook.com/infosys/cgi/issue.cgi#476 $

Copyright © Ravenbrook Limited. This document is provided "as is", without any express or implied warranty. In no event will the authors be held liable for any damages arising from the use of this document. You may not duplicate or reproduce this document in any form without the express permission of the copyright holder.

Ravenbrook / Projects / Memory Pool System / Issues