Skip to content

Commit

Permalink
Detected the issue with crash in HashTableBase::initTable();
Browse files Browse the repository at this point in the history 
It was because of using memset to initialize the table with all zeros.
The table was an array of KVPair objects; memory was allocated by operator new.

Instead, if we use malloc/free to allocate table, and use memset() to initialize it,
no problem occurs.

Need to test this in the main branch.
  • Loading branch information
saad0105050 committed Sep 16, 2014
1 parent 7b7002b commit a490f54
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 61 deletions.
1 change: 1 addition & 0 deletions YASI_12/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace yasi{
#define MAKE_UNIQUE3(x,y, z) MAKE_UNIQUE2( CONCAT(x,y), z )

#define DELETE_SAFE(x) {if( (x) != 0 ) delete (x); (x) = 0; }
#define FREE_SAFE(x) {if( (x) != 0 ) free(x); (x) = 0; }
#define DELETE_ARR_SAFE(x) {if( (x) != 0 ) delete[] (x); (x) = 0; }
#define ARR_LENGTH(x) ( sizeof( (x) )/sizeof( (x)[0] ) )

Expand Down
139 changes: 93 additions & 46 deletions YASI_12/ds.LinearProbingHashTable.h
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#pragma once
#include "common.h"
#include "ds.hashtablebase.h"
#include <cstring>
#include <cstdlib>
#include <algorithm>
using namespace std;

namespace yasi{
Expand All @@ -16,7 +19,7 @@ namespace ds{
class LinearProbingHashTable : public HashTableBase <
Key,
Value,
EntryType, // storing objects, not pointers
EntryType*, // storing objects, not pointers
HashFunction > {
///////////////// enable testing ///////////////////
friend class LinearProbingHashTableTest;
Expand All @@ -25,7 +28,7 @@ namespace ds{

//typedef KVPair<Key, Value> Pair;
//typedef EntryType Pair; // technically, this pair may not have a value
typedef EntryType BucketType; // each bucket holds an EntryType object
typedef EntryType* BucketType; // each bucket holds an EntryType object
typedef EntryType Pair; // which is actually a key-value pair
typedef EntryType* BucketEntryPtr; // ptr to an entry object
typedef Key KeyType;
Expand All @@ -37,6 +40,7 @@ namespace ds{
char* _pZeroKey; // all zeros up to #bytes of a Key; used to check if a Key is zero
char* _pZeroValue; // all zeros up to #bytes of a Value; used to set a Value to zero

#if 0
inline unsigned int circularNext(const int index) const{
return modSize(index + 1);
}
Expand Down Expand Up @@ -160,23 +164,6 @@ namespace ds{
}
}

virtual void copyTable(BucketType* oldTable, const unsigned int oldSize) override {
// the zeroPair stays intact because it is determined by key, not hash value
// copy table elements
//BucketType* oldTable = (BucketType*)prevTable;
_population = _zeroUsed ? 1 : 0;

for (unsigned int i = 0; i < oldSize; i++){
EntryType p = oldTable[i];
if ( !isKeyZero(&p.key)){
insert(p.key, p.value);
// no need to delete because it is not pointer anymore
//DELETE_SAFE(oldTable[i]);
}
}
// count the zero element

}

// forecefully provide a table (and associated state values)
// for test purpose only
Expand All @@ -188,16 +175,8 @@ namespace ds{
_population = newPopulation;
hash = newHashFunction;
}
void clear(){
//if (table){
// for (unsigned int i = 0; i < _size; i++){
// removeEntry(i);
// }
//}
// DELETE_ARR_SAFE(table);
_zeroUsed = false;
_size = _population = _logSize = 0;
}



////////////////////////////////////////////////
//// abstracting the data type of the table
Expand Down Expand Up @@ -243,35 +222,69 @@ namespace ds{
table[bucket] = Pair(k);
// if we stored pointers, we would have done new Pair(k)
}
#endif
virtual bool isBucketEmpty(const int bucket) const override {
return isNull(bucket);
return false; // return isNull(bucket);
}
virtual void copyTable(BucketType* oldTable, const unsigned int oldSize) override {
//// the zeroPair stays intact because it is determined by key, not hash value
//// copy table elements
////BucketType* oldTable = (BucketType*)prevTable;
//_population = _zeroUsed ? 1 : 0;
//
//for (unsigned int i = 0; i < oldSize; i++){
// EntryType p = oldTable[i];
// if ( !isKeyZero(&p.key)){
// insert(p.key, p.value);
// // no need to delete because it is not pointer anymore
// //DELETE_SAFE(oldTable[i]);
// }
//}
//// count the zero element

}

void clear(){
//if (table){
// for (unsigned int i = 0; i < _size; i++){
// removeEntry(i);
// }
//}
// DELETE_ARR_SAFE(table);
_zeroUsed = false;
_size = _population = _logSize = 0;
}

public:

LinearProbingHashTable(unsigned int logSize = INIT_LOGSIZE) : _zeroUsed(false), HashTableBase(logSize){
_pZeroKey = new char[sizeof(Key)];
_pZeroValue = new char[sizeof(Value)];
//_pZeroKey = new char[sizeof(Key)];
//_pZeroValue = new char[sizeof(Value)];

memset(_pZeroKey, 0, sizeof(Key));
//memset(_pZeroKey, 0, sizeof(Key));

setKeyZero(&_zeroKeyEntry.key);
//setKeyZero(&_zeroKeyEntry.key);
}
virtual ~LinearProbingHashTable(){
clear();
DELETE_SAFE(_pZeroKey);
DELETE_SAFE(_pZeroValue);
//clear();
//DELETE_SAFE(_pZeroKey);
//DELETE_SAFE(_pZeroValue);
}
virtual Value* get(const Key& key) const override {
Pair* kv = lookupKey(key);
if (kv)
return &(kv->value);
else
//Pair* kv = lookupKey(key);
//if (kv)
// return &(kv->value);
//else
return NULL;
}

virtual void put(const Key& key, const Value& value) override { return insert(key, value); }
virtual bool contains(const Key& key) const override { return lookupKey(key) != NULL; }
virtual void put(const Key& key, const Value& value) override {
//return insert(key, value);
}
virtual bool contains(const Key& key) const override {
return false; // lookupKey(key) != NULL;
}
virtual void remove(const Key& k) override {
#if 0
// zero key
if (isKeyZero(&k)){
if (_zeroUsed) {
Expand Down Expand Up @@ -359,6 +372,8 @@ namespace ds{
// cannot remove; done
}
return;
#endif

}

};
Expand Down Expand Up @@ -425,6 +440,7 @@ namespace ds{


public:
#if 0
void insert(){

IntHashTable h(4);
Expand Down Expand Up @@ -884,7 +900,7 @@ namespace ds{
ASSERT_EQ(popLimit - 1, h1.population()) << "h1 pop not " << popLimit - 1;
}
}

#endif
void isKeyZero(){
//struct MyStruct{
// int a;
Expand Down Expand Up @@ -931,7 +947,38 @@ namespace ds{
//h.makeEntry(4, 44);
//ASSERT_EQ(44, h.key(4)) << "[4] not 44";

ASSERT_EQ(0, 1) << "uncomment other tests";

{
class A{
public:
virtual void x() = 0;
};

class BB :public A{
public:
int key;
int value;
void x() override {}
};

//typedef BB B;
//typedef KVPairSimple<int> B;
typedef KVPair<int> B;
int n = 1;
int s = sizeof(B);
B* pArr = (B*)malloc(n * s);
//B* pArr = new B[n];
memset(pArr, 0, s * n);
//std::fill((char*)pArr, (char*)(pArr+n), 0);
//for (int i = 0; i < n; i++){
// pArr[i].key = 0xAAAAAAAA;
// pArr[i].value = 0xBBBBBBBB;
//}
//delete[] pArr;
free(pArr);
}

//ASSERT_EQ(0, 1) << "uncomment other tests";
}
};

Expand Down
14 changes: 8 additions & 6 deletions YASI_12/ds.hashtablebase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ class HashTableBase : public IKeyValueStore < Key, Value >{
BucketType& bucket(const Key& k) const{
return table[index(k)];
}
void initTable(BucketType** pTable, const int numElems){
void initTable(BucketType* pTable, const int numElems){
// initialize to zero
const int sizeOfEntry = sizeof(BucketType);
memset(*pTable, 0, numElems * sizeOfEntry);
memset(pTable, 0, numElems * sizeOfEntry);
}
inline float density(){ return ((float) _population) / _size; }
// true if the specified population would be too dense for current table
Expand Down Expand Up @@ -133,7 +133,7 @@ class HashTableBase : public IKeyValueStore < Key, Value >{
_size = newSize;
_population = 0;
table = new BucketType[newSize]; // twice the current size
initTable(&table, newSize); // initialize with zero
initTable(table, newSize); // initialize with zero
// copy old elements
// copy table elements
copyTable(oldTable, oldSize);
Expand All @@ -145,11 +145,13 @@ class HashTableBase : public IKeyValueStore < Key, Value >{
HashTableBase():HashTableBase(INIT_LOGSIZE){}
HashTableBase(unsigned int logSize = INIT_LOGSIZE) : _logSize(logSize), _size(1 << logSize), _population(0), DENSITY_MAX(0.75), DENSITY_MIN(0.25){
assert(_logSize > 0);
table = new BucketType[_size];
initTable(&table, _size);
//table = new BucketType[_size];
table = (BucketType*)malloc(_size * sizeof(BucketType));
initTable(table, _size);
}
virtual ~HashTableBase(){
DELETE_ARR_SAFE(table);
//DELETE_ARR_SAFE(table);
FREE_SAFE(table);
_size = _population = _logSize = 0;
}

Expand Down
36 changes: 36 additions & 0 deletions YASI_12/ds.kvpair.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,42 @@ namespace yasi{
namespace ds{
using namespace std;

class A{};
// the basic key-value pair class
// does not implement IComparable interface
// this is important, because it makes the class simple (without vtable)
// the key-value pair class
template<class Key, class Value = Key>
struct KVPairSimple //: public A
{
typedef KVPairSimple<Key, Value> self;
public:
Key key;
Value value;

//virtual ~KVPairSimple(){}
//KVPairSimple(){}
//KVPairSimple(Key k, Value v) : key(k), value(v){}
//explicit KVPairSimple(Key k) : key(k){}
//KVPairSimple(const self& other) :key(other.key), value(other.value){}
//self& operator= (const self& other){ key = other.key; value = other.value; return *this; }
//inline bool operator==(const self& other) const { return key == other.key; }
//inline bool operator< (const self& other) const { return key < other.key; }
//inline bool operator<= (const self& other) const { return key <= other.key; }
//inline bool operator!=(const self& other) const { return !((*this) == other); }
};
// override for printing
template<class Key, class Value>
std::ostream& operator<< (std::ostream& out, const KVPairSimple<Key, Value>& kv) {
out << kv.key;
return out;
}
template<class Key, class Value>
std::ostream& operator<< (std::ostream& out, const KVPairSimple<Key, Value>* pKv) {
out << pKv->key;
return out;
}

// the key-value pair class
template<class Key, class Value = Key>
class KVPair :public IComparable< KVPair<Key, Value> >{
Expand Down
18 changes: 9 additions & 9 deletions YASI_12/main.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#include "common.h"
#include "ds.singlylinkedlist.h"
#include "ds.doublylinkedlist.h"
#include "ds.arraybinarytree.h"
#include "ds.binaryheap.h"
#include "ds.binarytree.h"
#include "ds.binarysearchtree.h"
#include "ds.priorityqueue.h"
#include "ds.BSTDictionary.h"
#include "ds.separatechaininghashtable.h"
//#include "ds.singlylinkedlist.h"
//#include "ds.doublylinkedlist.h"
//#include "ds.arraybinarytree.h"
//#include "ds.binaryheap.h"
//#include "ds.binarytree.h"
//#include "ds.binarysearchtree.h"
//#include "ds.priorityqueue.h"
//#include "ds.BSTDictionary.h"
//#include "ds.separatechaininghashtable.h"
#include "ds.linearprobinghashtable.h"
//#include "Sorter.h"

Expand Down

0 comments on commit a490f54

Please sign in to comment.