> The above change leaves n_files_in_map uninitialized.
>
> In general, please initialize members before entering the body of the
> constructor. This will save you from bugs such as this one and will also
> make the code more exception-safe.
Done.
>> +/** Compact bit-field representation class
>> + *
>> + * FileMap is aimed at holding UFS's internal file allocation table.
>
> I do not think FileMap is a compact representation of a bit-field, but
> perhaps you meant something else? Consider this clarification:
>
> /** A bitmap used for managing UFS StoreEntry "file numbers".
> *
> * Nth bit represents whether file number N is used.
> * The map automatically grows to hold up to 2^24 bits.
> * New bit is "off" or zero by default, representing unused fileno.
> */
Thanks. Done.
>
>> + /// Test whether the num-th bit in the FileMap is set
>> + int testBit(int num) const;
>
> I would rename that to isUsed(), isSet() or similar and return bool
> instead of int, but that is not a big deal.
Leaving as-is not to grow the scope.
>> + // max number of files which can be tracked in the current store
>> + int max_n_files;
>> + int n_files_in_map; // used slots in the map
>> + int nwords; // number of "long ints" making up the filemap
>> + unsigned long *file_map;
>
> Please use doxygen comments for data members and document file_map as well.
Done.
> I would also s/file_map/words/ or similar (to avoid "file map inside a
> file map" implication) but that is your call.
Renamed as "bitmap". Is that OK?
> I would also add the following to-do, perhaps where file_map data member
> is declared:
>
> // TODO: Consider using std::bitset instead.
Done.
Version 2 attached.
Thanks
-- /kinkie
This archive was generated by hypermail 2.2.0 : Sat Nov 26 2011 - 12:00:07 MST