Skip to content

Commit c357227

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 3ef8386 commit c357227

2 files changed

Lines changed: 20 additions & 20 deletions

File tree

Hashtable.c

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

@@ -208,21 +213,18 @@ static void insert(Hashtable* this, ht_key_t key, void* value) {
208213
}
209214
}
210215

211-
void Hashtable_setSize(Hashtable* this, size_t size) {
212-
216+
static void Hashtable_setSize(Hashtable* this, size_t size) {
213217
assert(Hashtable_isConsistent(this));
218+
assert(size >= this->items);
214219

215-
if (size <= this->items)
216-
return;
217-
218-
size_t newSize = nextPrime(size);
219-
if (newSize == this->size)
220+
size = nextPrime(size);
221+
if (size == this->size)
220222
return;
221223

222224
HashtableItem* oldBuckets = this->buckets;
223225
size_t oldSize = this->size;
224226

225-
this->size = newSize;
227+
this->size = size;
226228
this->buckets = (HashtableItem*) xCalloc(this->size, sizeof(HashtableItem));
227229
this->items = 0;
228230

@@ -240,24 +242,20 @@ void Hashtable_setSize(Hashtable* this, size_t size) {
240242
}
241243

242244
void Hashtable_put(Hashtable* this, ht_key_t key, void* value) {
243-
244245
assert(Hashtable_isConsistent(this));
245-
assert(this->size > 0);
246246
assert(value);
247247

248248
/* grow on load-factor > 0.7 */
249-
if (10 * this->items > 7 * this->size) {
250-
if (SIZE_MAX / 2 < this->size)
251-
CRT_fatalError("Hashtable: size overflow");
249+
if (sizeof(HashtableItem) < 7 && SIZE_MAX / 7 < this->size)
250+
CRT_fatalError("Hashtable: size overflow");
252251

253-
Hashtable_setSize(this, 2 * this->size);
254-
}
252+
if (this->items >= this->size * 7 / 10)
253+
Hashtable_setSize(this, this->size + 1);
255254

256255
insert(this, key, value);
257256

258257
assert(Hashtable_isConsistent(this));
259258
assert(Hashtable_get(this, key) != NULL);
260-
assert(this->size > this->items);
261259
}
262260

263261
void* Hashtable_remove(Hashtable* this, ht_key_t key) {
@@ -309,8 +307,12 @@ void* Hashtable_remove(Hashtable* this, ht_key_t key) {
309307
assert(Hashtable_get(this, key) == NULL);
310308

311309
/* shrink on load-factor < 0.125 */
312-
if (8 * this->items < this->size)
313-
Hashtable_setSize(this, this->size / 3); /* account for nextPrime rounding up */
310+
if (sizeof(HashtableItem) < 3 && SIZE_MAX / 3 < this->size)
311+
CRT_fatalError("Hashtable: size overflow");
312+
313+
if (this->items < this->size / 8) {
314+
Hashtable_setSize(this, this->size * 3 / 8); /* account for nextPrime rounding up */
315+
}
314316

315317
return res;
316318
}

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)