opened
sunheehnus
username_0
(In dictc branch) As we directly touch rehashidx, it's better we keep it in synchronization.
dict.c/dictRehash: check again to update
issue
created
antirez
username_1
Good catch! rehashidx could be outside the table size otherwise, which does not look sane now that we are actively using it in other parts of the dict.c implementation. Btw what about this implementation instead?


```diff
diff --git a/src/dict.c b/src/dict.c
index 9ba0d7d..c8aaf15 100644
--- a/src/dict.c
+++ b/src/dict.c
@@ -246,18 +246,9 @@ int dictRehash(dict *d, int n) {
     int empty_visits = n*10; /* Max number of empty buckets to visit. */
     if (!dictIsRehashing(d)) return 0;

-    while(n--) {
+    while(n-- && d->ht[0].used != 0) {
         dictEntry *de, *nextde;

-        /* Check if we already rehashed the whole table... */
-        if (d->ht[0].used == 0) {
-            zfree(d->ht[0].table);
-            d->ht[0] = d->ht[1];
-            _dictReset(&d->ht[1]);
-            d->rehashidx = -1;
-            return 0;
-        }
-
         /* Note that rehashidx can't overflow as we are sure there are more
          * elements because ht[0].used != 0 */
         assert(d->ht[0].size > (unsigned long)d->rehashidx);
@@ -282,6 +273,17 @@ int dictRehash(dict *d, int n) {
         d->ht[0].table[d->rehashidx] = NULL;
         d->rehashidx++;
     }
+
+    /* Check if we already rehashed the whole table... */
+    if (d->ht[0].used == 0) {
+        zfree(d->ht[0].table);
+        d->ht[0] = d->ht[1];
+        _dictReset(&d->ht[1]);
+        d->rehashidx = -1;
+        return 0;
+    }
+
+    /* More to rehash... */
     return 1;
 }
```
comment
created
antirez
username_1
I merged your PR and committed acefdff to avoid the code duplication. Should be equivalent. Also now we correctly return 0 in all cases when the rehashing is terminated. Thanks.
comment
created
sunheehnus
username_0
@username_1  Your patch is a better fix  :+1:  :+1:   :-)
comment