On Sun, 27 Nov 2011 19:55:51 +0100, Kinkie wrote:
<snip>
* n_files_in_map would be best called something_ internally. Also
this
should be unsigned.
changed to sfileno used_slots_
Err... camelCase_
<snip>
NP: also this would be a good time to add that bounds checking and
ensure
the n_files_in_map is correctly maintained by its owner class.
Especially
since its a private. This goes for both setBit() and checkBit(). The
return
value from setBit() appears not to be used anywhere, so both can
change to
bool and report back success/fail to the caller in case it is ever
needed.
This would go in the "improvements" part.
Okay, up to you.
With those mentioned changes its passed me.
Also, there is a bit of
layering breaking in UFSSwapDir where it has its own view of the map,
including e.g. keeping a cached "last allocated" index to speed
allocations up.
I'd prefer not to address these at this time, but I could add a TODO
to remind about them if you wish.
V3 attached.
I would not really call that "suggestion" variable layer breaking. UFS
is not manipulating it directly AFAICS. Just passing back to the FileMap
later. Which IMO is a correct implementation of the temporality
optimization.
The StoreMap could be considered a layer break in a certain view. I
agree that is way out of scope here though. So far out I'd argue its out
of scope of improving FileMap too.
Amos
