Skip to content

Commit 80c8562

Browse files
committed
Improve Hashtable buffer grow and shrink conditions
* Move assertions about hash table sizes to Hashtable_isConsistent() so they can be checked in all Hashtable methods. * Slightly improve conditionals of growing and shrinking the "buckets" buffer. Specifically the calculations are now less prone to arithmetic overflow and can work with Hashtable.size value up to (SIZE_MAX / 7). (Original limit was (SIZE_MAX / 10)). * If `Hashtable.size > SIZE_MAX / sizeof(HashtableItem)`, allow the compiler to optimize out one conditional of checking overflow. (The buffer allocation would still fail at xCalloc() in that case.) * Hashtable_setSize() is now a private method. Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
1 parent b63412e commit 80c8562

File tree

2 files changed

+20
-20
lines changed

2 files changed

+20
-20
lines changed

Hashtable.c

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ static bool Hashtable_isConsistent(const Hashtable* this) {
7474
bool res = items == this->items;
7575
if (!res)
7676
Hashtable_dump(this);
77+
78+
assert(this->size > 0);
79+
assert(this->size <= SIZE_MAX / sizeof(HashtableItem));
80+
assert(this->size >= this->items);
81+
7782
return res;
7883
}
7984

@@ -187,21 +192,18 @@ static void insert(Hashtable* this, ht_key_t key, void* value) {
187192
}
188193
}
189194

190-
void Hashtable_setSize(Hashtable* this, size_t size) {
191-
195+
static void Hashtable_setSize(Hashtable* this, size_t size) {
192196
assert(Hashtable_isConsistent(this));
197+
assert(size >= this->items);
193198

194-
if (size <= this->items)
195-
return;
196-
197-
size_t newSize = nextPrime(size);
198-
if (newSize == this->size)
199+
size = nextPrime(size);
200+
if (size == this->size)
199201
return;
200202

201203
HashtableItem* oldBuckets = this->buckets;
202204
size_t oldSize = this->size;
203205

204-
this->size = newSize;
206+
this->size = size;
205207
this->buckets = (HashtableItem*) xCalloc(this->size, sizeof(HashtableItem));
206208
this->items = 0;
207209

@@ -219,24 +221,20 @@ void Hashtable_setSize(Hashtable* this, size_t size) {
219221
}
220222

221223
void Hashtable_put(Hashtable* this, ht_key_t key, void* value) {
222-
223224
assert(Hashtable_isConsistent(this));
224-
assert(this->size > 0);
225225
assert(value);
226226

227227
/* grow on load-factor > 0.7 */
228-
if (10 * this->items > 7 * this->size) {
229-
if (SIZE_MAX / 2 < this->size)
230-
CRT_fatalError("Hashtable: size overflow");
228+
if (sizeof(HashtableItem) < 7 && SIZE_MAX / 7 < this->size)
229+
CRT_fatalError("Hashtable: size overflow");
231230

232-
Hashtable_setSize(this, 2 * this->size);
233-
}
231+
if (this->items >= this->size * 7 / 10)
232+
Hashtable_setSize(this, this->size + 1);
234233

235234
insert(this, key, value);
236235

237236
assert(Hashtable_isConsistent(this));
238237
assert(Hashtable_get(this, key) != NULL);
239-
assert(this->size > this->items);
240238
}
241239

242240
void* Hashtable_remove(Hashtable* this, ht_key_t key) {
@@ -288,8 +286,12 @@ void* Hashtable_remove(Hashtable* this, ht_key_t key) {
288286
assert(Hashtable_get(this, key) == NULL);
289287

290288
/* shrink on load-factor < 0.125 */
291-
if (8 * this->items < this->size)
292-
Hashtable_setSize(this, this->size / 3); /* account for nextPrime rounding up */
289+
if (sizeof(HashtableItem) < 3 && SIZE_MAX / 3 < this->size)
290+
CRT_fatalError("Hashtable: size overflow");
291+
292+
if (this->items < this->size / 8) {
293+
Hashtable_setSize(this, this->size * 3 / 8); /* account for nextPrime rounding up */
294+
}
293295

294296
return res;
295297
}

Hashtable.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ void Hashtable_delete(Hashtable* this);
2929

3030
void Hashtable_clear(Hashtable* this);
3131

32-
void Hashtable_setSize(Hashtable* this, size_t size);
33-
3432
void Hashtable_put(Hashtable* this, ht_key_t key, void* value);
3533

3634
void* Hashtable_remove(Hashtable* this, ht_key_t key);

0 commit comments

Comments
 (0)