Commit d5ffabcd authored by Mark Olesen's avatar Mark Olesen
Browse files

HashTbl - extra safety

- don't let automatic resize into integer overflow
- avoid possible overflow in increment()

StaticHashTable - fix erase()/++ as per HashTbl

- might still be worth dropping it, but at least it'll be in better
  condition when it gets chucked!
parent 39159a92
......@@ -63,7 +63,7 @@ primitives/random/Random.C
containers/HashTables/HashTbl/HashTblCore.C
containers/HashTables/HashTable/HashTableName.C
containers/HashTables/StaticHashTable/StaticHashTableName.C
containers/HashTables/StaticHashTable/StaticHashTableCore.C
containers/Lists/SortableList/ParSortableListName.C
containers/Lists/PackedList/PackedListName.C
containers/Lists/ListOps/ListOps.C
......
......@@ -227,25 +227,25 @@ Foam::HashTbl<T, Key, Hash>::find
template<class T, class Key, class Hash>
Foam::List<Key> Foam::HashTbl<T, Key, Hash>::toc() const
{
List<Key> tofc(nElmts_);
label i = 0;
List<Key> keys(nElmts_);
label keyI = 0;
for (const_iterator iter = cbegin(); iter != cend(); ++iter)
{
tofc[i++] = iter.key();
keys[keyI++] = iter.key();
}
return tofc;
return keys;
}
template<class T, class Key, class Hash>
Foam::List<Key> Foam::HashTbl<T, Key, Hash>::sortedToc() const
{
List<Key> sortedList = this->toc();
sort(sortedList);
List<Key> sortedLst = this->toc();
sort(sortedLst);
return sortedList;
return sortedLst;
}
......@@ -283,7 +283,7 @@ bool Foam::HashTbl<T, Key, Hash>::set
table_[hashIdx] = new hashedEntry(key, table_[hashIdx], newEntry);
nElmts_++;
if (double(nElmts_)/tableSize_ > 0.8)
if (double(nElmts_)/tableSize_ > 0.8 && tableSize_ < maxTableSize)
{
# ifdef FULLDEBUG
if (debug)
......@@ -377,11 +377,11 @@ bool Foam::HashTbl<T, Key, Hash>::iteratorBase::erase()
// Mark with special hashIndex value to signal it has been rewound.
// The next increment will bring it back to the present location.
//
// For the current position 'X', mark it as '-(X+1)', which is
// written as '-X-1' to avoid overflow.
// The extra '-1' is needed to avoid ambiguity for position '0'.
// To retrieve the previous position 'X-1' we would later
// use '-(-X-1) - 2'
// From the current position 'curPos', we wish to continue at
// prevPos='curPos-1', which we mark as markPos='-curPos-1'.
// The negative lets us notice it is special, the extra '-1'
// is needed to avoid ambiguity for position '0'.
// To retrieve prevPos, we would later use '-(markPos+1) - 1'
hashIndex_ = -hashIndex_ - 1;
}
......
......@@ -80,6 +80,9 @@ struct HashTblCore
//- Return a canonical (power-of-two) size
static label canonicalSize(const label);
//- Maximum allowable table size
static const label maxTableSize;
//- Construct null
HashTblCore()
{}
......@@ -288,7 +291,7 @@ public:
void transfer(HashTbl<T, Key, Hash>&);
//- Transfer contents to the Xfer container
inline Xfer<HashTbl<T, Key, Hash> > xfer();
inline Xfer< HashTbl<T, Key, Hash> > xfer();
// Member Operators
......
......@@ -30,4 +30,12 @@ License
defineTypeNameAndDebug(Foam::HashTblCore, 0);
const Foam::label Foam::HashTblCore::maxTableSize
(
Foam::HashTblCore::canonicalSize
(
Foam::labelMax/2
)
);
// ************************************************************************* //
......@@ -221,9 +221,9 @@ Foam::HashTbl<T, Key, Hash>::iteratorBase::increment()
// A negative index is a special value from erase
if (hashIndex_ < 0)
{
// old position 'X' was marked as '-(X+1)'
// but we wish to continue at 'X-1'
hashIndex_ = -hashIndex_ - 2;
// the markPos='-curPos-1', but we wish to continue at 'curPos-1'
// thus use '-(markPos+1) -1'
hashIndex_ = -(hashIndex_+1) - 1;
}
else if (entryPtr_)
{
......
......@@ -33,8 +33,7 @@ License
// * * * * * * * * * * * * Private Member Functions * * * * * * * * * * * * //
template<class T, class Key, class Hash>
Foam::label Foam::StaticHashTable<T, Key, Hash>::canonicalSize(const label size)
Foam::label Foam::StaticHashTableCore::canonicalSize(const label size)
{
if (size < 1)
{
......@@ -64,8 +63,8 @@ Foam::label Foam::StaticHashTable<T, Key, Hash>::canonicalSize(const label size)
template<class T, class Key, class Hash>
Foam::StaticHashTable<T, Key, Hash>::StaticHashTable(const label size)
:
StaticHashTableName(),
keys_(canonicalSize(size)),
StaticHashTableCore(),
keys_(StaticHashTableCore::canonicalSize(size)),
objects_(keys_.size()),
nElmts_(0),
endIter_(*this, keys_.size(), 0),
......@@ -89,7 +88,7 @@ Foam::StaticHashTable<T, Key, Hash>::StaticHashTable
const StaticHashTable<T, Key, Hash>& ht
)
:
StaticHashTableName(),
StaticHashTableCore(),
keys_(ht.keys_),
objects_(ht.objects_),
nElmts_(ht.nElmts_),
......@@ -105,7 +104,7 @@ Foam::StaticHashTable<T, Key, Hash>::StaticHashTable
const Xfer< StaticHashTable<T, Key, Hash> >& ht
)
:
StaticHashTableName(),
StaticHashTableCore(),
keys_(0),
objects_(0),
nElmts_(0),
......@@ -224,15 +223,15 @@ Foam::StaticHashTable<T, Key, Hash>::find
template<class T, class Key, class Hash>
Foam::List<Key> Foam::StaticHashTable<T, Key, Hash>::toc() const
{
List<Key> tofc(nElmts_);
label i = 0;
List<Key> keys(nElmts_);
label keyI = 0;
for (const_iterator iter = cbegin(); iter != cend(); ++iter)
{
tofc[i++] = iter.key();
keys[keyI++] = iter.key();
}
return tofc;
return keys;
}
......@@ -319,24 +318,9 @@ bool Foam::StaticHashTable<T, Key, Hash>::erase(const iterator& cit)
if (it.elemIndex_ < 0)
{
// No previous element in the local list
// Search back for previous non-zero table entry
while (--it.hashIndex_ >= 0 && !objects_[it.hashIndex_].size())
{}
if (it.hashIndex_ >= 0)
{
// The last element in the local list
it.elemIndex_ = objects_[it.hashIndex_].size() - 1;
}
else
{
// No previous found. Mark with special value which is
// - not end()
// - handled by operator++
it.hashIndex_ = -1;
it.elemIndex_ = 0;
}
// Mark with as special value (see notes in HashTable)
it.hashIndex_ = -it.hashIndex_ - 1;
it.elemIndex_ = 0;
}
nElmts_--;
......@@ -407,8 +391,8 @@ Foam::label Foam::StaticHashTable<T, Key, Hash>::erase
template<class T, class Key, class Hash>
void Foam::StaticHashTable<T, Key, Hash>::resize(const label sz)
{
label newSize = canonicalSize(sz);
label newSize = StaticHashTableCore::canonicalSize(sz);
if (newSize == keys_.size())
{
# ifdef FULLDEBUG
......@@ -543,18 +527,8 @@ bool Foam::StaticHashTable<T, Key, Hash>::operator==
const StaticHashTable<T, Key, Hash>& rhs
) const
{
// Are all my elements in rhs?
for (const_iterator iter = cbegin(); iter != cend(); ++iter)
{
const_iterator fnd = rhs.find(iter.key());
if (fnd == rhs.cend() || fnd() != iter())
{
return false;
}
}
// sizes (number of keys) must match
// Are all rhs elements in me?
for (const_iterator iter = rhs.cbegin(); iter != rhs.cend(); ++iter)
{
const_iterator fnd = find(iter.key());
......@@ -564,6 +538,7 @@ bool Foam::StaticHashTable<T, Key, Hash>::operator==
return false;
}
}
return true;
}
......
......@@ -76,7 +76,33 @@ template<class T, class Key, class Hash> Ostream& operator<<
Class StaticHashTableName Declaration
\*---------------------------------------------------------------------------*/
TemplateName(StaticHashTable);
/*---------------------------------------------------------------------------*\
Class StaticHashTableCore Declaration
\*---------------------------------------------------------------------------*/
//- Template-invariant bits for StaticHashTable
struct StaticHashTableCore
{
//- Return a canonical (power-of-two) size
static label canonicalSize(const label);
//- Construct null
StaticHashTableCore()
{}
//- Define template name and debug
ClassName("StaticHashTable");
//- A zero-sized end iterator
struct iteratorEnd
{
//- Construct null
iteratorEnd()
{}
};
};
/*---------------------------------------------------------------------------*\
......@@ -86,7 +112,7 @@ TemplateName(StaticHashTable);
template<class T, class Key=word, class Hash=string::hash>
class StaticHashTable
:
public StaticHashTableName
public StaticHashTableCore
{
// Private data type for table entries
......
......@@ -28,6 +28,6 @@ License
// * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //
defineTypeNameAndDebug(Foam::StaticHashTableName, 0);
defineTypeNameAndDebug(Foam::StaticHashTableCore, 0);
// ************************************************************************* //
......@@ -267,8 +267,13 @@ Foam::StaticHashTable<T, Key, Hash>::Iterator
TableRef
>::operator++()
{
// Check for special value from erase. (sets hashIndex to -1)
if (hashIndex_ >= 0)
// A negative index is a special value from erase
// (see notes in HashTable)
if (hashIndex_ < 0)
{
hashIndex_ = -(hashIndex_+1) - 1;
}
else
{
// Try the next element on the local list
elemIndex_++;
......
......@@ -37,9 +37,9 @@ Foam::StaticHashTable<T, Key, Hash>::StaticHashTable
const label size
)
:
StaticHashTableName(),
keys_(size),
objects_(size),
StaticHashTableCore(),
keys_(StaticHashTableCore::canonicalSize(size)),
objects_(StaticHashTableCore::canonicalSize(size)),
nElmts_(0),
endIter_(*this, keys_.size(), 0),
endConstIter_(*this, keys_.size(), 0)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment