Lazyfree: Hash converted to use plain SDS WIP 4.

9 files changed, 76 insertions, 70 deletions

diff --git a/src/aof.c b/src/aof.c
index 3ab6c85b2..4d971ff10 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -928,17 +928,13 @@ static int rioWriteHashIteratorCursor(rio *r, hashTypeIterator *hi, int what) {
         long long vll = LLONG_MAX;
 
         hashTypeCurrentFromZiplist(hi, what, &vstr, &vlen, &vll);
-        if (vstr) {
+        if (vstr)
             return rioWriteBulkString(r, (char*)vstr, vlen);
-        } else {
+        else
             return rioWriteBulkLongLong(r, vll);
-        }
-
     } else if (hi->encoding == OBJ_ENCODING_HT) {
-        robj *value;
-
-        hashTypeCurrentFromHashTable(hi, what, &value);
-        return rioWriteBulkObject(r, value);
+        sds value = hashTypeCurrentFromHashTable(hi, what);
+        return rioWriteBulkString(r, value, sdslen(value));
     }
 
     serverPanic("Unknown hash encoding");
diff --git a/src/db.c b/src/db.c
index 893bc96c4..d4b215e53 100644
--- a/src/db.c
+++ b/src/db.c
@@ -420,11 +420,11 @@ void scanCallback(void *privdata, const dictEntry *de) {
     } else if (o->type == OBJ_HASH) {
         sds sdskey = dictGetKey(de);
         sds sdsval = dictGetVal(de);
-        key = createStringObject(keysds,sdslen(keysds));
-        val = createStringObject(valsds,sdslen(valsds));
+        key = createStringObject(sdskey,sdslen(sdskey));
+        val = createStringObject(sdsval,sdslen(sdsval));
     } else if (o->type == OBJ_ZSET) {
-        sds keysds = dictGetKey(de);
-        key = createStringObject(keysds,sdslen(keysds));
+        sds sdskey = dictGetKey(de);
+        key = createStringObject(sdskey,sdslen(sdskey));
         val = createStringObjectFromLongDouble(*(double*)dictGetVal(de),0);
     } else {
         serverPanic("Type not handled in SCAN callback.");
diff --git a/src/debug.c b/src/debug.c
index 96d36f2eb..463b7f374 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -222,20 +222,18 @@ void computeDatasetDigest(unsigned char *final) {
                     serverPanic("Unknown sorted set encoding");
                 }
             } else if (o->type == OBJ_HASH) {
-                hashTypeIterator *hi;
-                robj *obj;
-
-                hi = hashTypeInitIterator(o);
+                hashTypeIterator *hi = hashTypeInitIterator(o);
                 while (hashTypeNext(hi) != C_ERR) {
                     unsigned char eledigest[20];
+                    sds sdsele;
 
                     memset(eledigest,0,20);
-                    obj = hashTypeCurrentObject(hi,OBJ_HASH_KEY);
-                    mixObjectDigest(eledigest,obj);
-                    decrRefCount(obj);
-                    obj = hashTypeCurrentObject(hi,OBJ_HASH_VALUE);
-                    mixObjectDigest(eledigest,obj);
-                    decrRefCount(obj);
+                    sdsele = hashTypeCurrentObjectNewSds(hi,OBJ_HASH_KEY);
+                    mixDigest(eledigest,sdsele,sdslen(sdsele));
+                    sdsfree(sdsele);
+                    sdsele = hashTypeCurrentObjectNewSds(hi,OBJ_HASH_VALUE);
+                    mixDigest(eledigest,sdsele,sdslen(sdsele));
+                    sdsfree(sdsele);
                     xorDigest(digest,eledigest,20);
                 }
                 hashTypeReleaseIterator(hi);
diff --git a/src/object.c b/src/object.c
index 43c371ead..167290d49 100644
--- a/src/object.c
+++ b/src/object.c
@@ -613,7 +613,7 @@ int strict_strtoll(char *str, long long *vp) {
     long long value;
 
     errno = 0;
-    value = strtoll(o->ptr, &eptr, 10);
+    value = strtoll(str, &eptr, 10);
     if (isspace(str[0]) || eptr[0] != '\0' || errno == ERANGE) return C_ERR;
     if (vp) *vp = value;
     return C_OK;
diff --git a/src/server.c b/src/server.c
index eff8afb2a..98c915a3d 100644
--- a/src/server.c
+++ b/src/server.c
@@ -607,7 +607,7 @@ dictType hashDictType = {
     dictSdsHash,                /* hash function */
     NULL,                       /* key dup */
     NULL,                       /* val dup */
-    dictsdsKeyCompare,          /* key compare */
+    dictSdsKeyCompare,          /* key compare */
     dictSdsDestructor,          /* key destructor */
     dictSdsDestructor           /* val destructor */
 };
diff --git a/src/server.h b/src/server.h
index ff5d6432f..e4d558f8b 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1333,10 +1333,9 @@ void setTypeConvert(robj *subject, int enc);
 void hashTypeConvert(robj *o, int enc);
 void hashTypeTryConversion(robj *subject, robj **argv, int start, int end);
 void hashTypeTryObjectEncoding(robj *subject, robj **o1, robj **o2);
-robj *hashTypeGetObject(robj *o, robj *key);
-int hashTypeExists(robj *o, robj *key);
-int hashTypeSet(robj *o, robj *key, robj *value);
-int hashTypeDelete(robj *o, robj *key);
+int hashTypeExists(robj *o, sds key);
+int hashTypeSet(robj *o, sds key, sds value);
+int hashTypeDelete(robj *o, sds key);
 unsigned long hashTypeLength(robj *o);
 hashTypeIterator *hashTypeInitIterator(robj *subject);
 void hashTypeReleaseIterator(hashTypeIterator *hi);
@@ -1345,9 +1344,11 @@ void hashTypeCurrentFromZiplist(hashTypeIterator *hi, int what,
                                 unsigned char **vstr,
                                 unsigned int *vlen,
                                 long long *vll);
-void hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what, robj **dst);
-robj *hashTypeCurrentObject(hashTypeIterator *hi, int what);
+sds hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what);
+void hashTypeCurrentObject(hashTypeIterator *hi, int what, unsigned char **vstr, unsigned int *vlen, long long *vll);
+sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what);
 robj *hashTypeLookupWriteOrCreate(client *c, robj *key);
+robj *hashTypeGetValueObject(robj *o, sds field);
 
 /* Pub / Sub */
 int pubsubUnsubscribeAllChannels(client *c, int notify);
diff --git a/src/sort.c b/src/sort.c
index 788f73a59..7ddd37d95 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -112,9 +112,9 @@ robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) {
     if (fieldobj) {
         if (o->type != OBJ_HASH) goto noobj;
 
-        /* Retrieve value from hash by the field name. This operation
-         * already increases the refcount of the returned object. */
-        o = hashTypeGetObject(o, fieldobj);
+        /* Retrieve value from hash by the field name. The returend object
+         * is a new object with refcount already incremented. */
+        o = hashTypeGetValueObject(o, fieldobj->ptr);
     } else {
         if (o->type != OBJ_STRING) goto noobj;
 
diff --git a/src/t_hash.c b/src/t_hash.c
index d3892f3b7..73ac12b75 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -67,7 +67,7 @@ int hashTypeGetFromZiplist(robj *o, sds field,
     zl = o->ptr;
     fptr = ziplistIndex(zl, ZIPLIST_HEAD);
     if (fptr != NULL) {
-        fptr = ziplistFind(fptr, field, sdslen(field), 1);
+        fptr = ziplistFind(fptr, (unsigned char*)field, sdslen(field), 1);
         if (fptr != NULL) {
             /* Grab pointer to the value (fptr points to the field) */
             vptr = ziplistNext(zl, fptr);
@@ -106,15 +106,15 @@ sds hashTypeGetFromHashTable(robj *o, sds field) {
  * If *vll is populated *vstr is set to NULL, so the caller
  * can always check the function return by checking the return value
  * for C_OK and checking if vll (or vstr) is NULL. */
-int hashTypeGetObject(robj *o, sds field, unsigned char **vstr, unsigned int *vlen, long long *vll) {
+int hashTypeGetValue(robj *o, sds field, unsigned char **vstr, unsigned int *vlen, long long *vll) {
     if (o->encoding == OBJ_ENCODING_ZIPLIST) {
         *vstr = NULL;
         if (hashTypeGetFromZiplist(o, field, vstr, vlen, vll) == 0)
             return C_OK;
     } else if (o->encoding == OBJ_ENCODING_HT) {
         sds value;
-        if (value = hashTypeGetFromHashTable(o, field) != NULL) {
-            *vstr = value;
+        if ((value = hashTypeGetFromHashTable(o, field)) != NULL) {
+            *vstr = (unsigned char*) value;
             *vlen = sdslen(value);
             return C_OK;
         }
@@ -124,6 +124,20 @@ int hashTypeGetObject(robj *o, sds field, unsigned char **vstr, unsigned int *vl
     return C_ERR;
 }
 
+/* Like hashTypeGetValue() but returns a Redis object, which is useful for
+ * interaction with the hash type outside t_hash.c.
+ * The function returns NULL if the field is not found in the hash. Otherwise
+ * a newly allocated string object with the value is returned. */
+robj *hashTypeGetValueObject(robj *o, sds field) {
+    unsigned char *vstr;
+    unsigned int vlen;
+    long long vll;
+
+    if (hashTypeGetValue(o,field,&vstr,&vlen,&vll) == C_ERR) return NULL;
+    if (vstr) return createStringObject((char*)vstr,vlen);
+    else return createStringObjectFromLongLong(vll);
+}
+
 /* Higher level function using hashTypeGet*() to return the length of the
  * object associated with the requested field, or 0 if the field does not
  * exist. */
@@ -157,8 +171,6 @@ int hashTypeExists(robj *o, sds field) {
 
         if (hashTypeGetFromZiplist(o, field, &vstr, &vlen, &vll) == 0) return 1;
     } else if (o->encoding == OBJ_ENCODING_HT) {
-        robj *aux;
-
         if (hashTypeGetFromHashTable(o, field) != NULL) return 1;
     } else {
         serverPanic("Unknown hash encoding");
@@ -178,7 +190,7 @@ int hashTypeSet(robj *o, sds field, sds value) {
         zl = o->ptr;
         fptr = ziplistIndex(zl, ZIPLIST_HEAD);
         if (fptr != NULL) {
-            fptr = ziplistFind(fptr, field, sdslen(field), 1);
+            fptr = ziplistFind(fptr, (unsigned char*)field, sdslen(field), 1);
             if (fptr != NULL) {
                 /* Grab pointer to the value (fptr points to the field) */
                 vptr = ziplistNext(zl, fptr);
@@ -189,14 +201,17 @@ int hashTypeSet(robj *o, sds field, sds value) {
                 zl = ziplistDelete(zl, &vptr);
 
                 /* Insert new value */
-                zl = ziplistInsert(zl, vptr, value->ptr, sdslen(value->ptr));
+                zl = ziplistInsert(zl, vptr, (unsigned char*)value,
+                        sdslen(value));
             }
         }
 
         if (!update) {
             /* Push new field/value pair onto the tail of the ziplist */
-            zl = ziplistPush(zl, field, sdslen(field), ZIPLIST_TAIL);
-            zl = ziplistPush(zl, value, sdslen(value), ZIPLIST_TAIL);
+            zl = ziplistPush(zl, (unsigned char*)field, sdslen(field),
+                    ZIPLIST_TAIL);
+            zl = ziplistPush(zl, (unsigned char*)value, sdslen(value),
+                    ZIPLIST_TAIL);
         }
         o->ptr = zl;
 
@@ -210,7 +225,7 @@ int hashTypeSet(robj *o, sds field, sds value) {
             dictGetVal(de) = sdsdup(value);
             update = 1;
         } else {
-            dictAdd(o->ptr,sdsdup(key),sdsdup(value));
+            dictAdd(o->ptr,sdsdup(field),sdsdup(value));
         }
     } else {
         serverPanic("Unknown hash encoding");
@@ -229,7 +244,7 @@ int hashTypeDelete(robj *o, sds field) {
         zl = o->ptr;
         fptr = ziplistIndex(zl, ZIPLIST_HEAD);
         if (fptr != NULL) {
-            fptr = ziplistFind(fptr, field, sdslen(field), 1);
+            fptr = ziplistFind(fptr, (unsigned char*)field, sdslen(field), 1);
             if (fptr != NULL) {
                 zl = ziplistDelete(zl,&fptr);
                 zl = ziplistDelete(zl,&fptr);
@@ -237,9 +252,6 @@ int hashTypeDelete(robj *o, sds field) {
                 deleted = 1;
             }
         }
-
-        decrRefCount(field);
-
     } else if (o->encoding == OBJ_ENCODING_HT) {
         if (dictDelete((dict*)o->ptr, field) == C_OK) {
             deleted = 1;
@@ -376,7 +388,7 @@ void hashTypeCurrentObject(hashTypeIterator *hi, int what, unsigned char **vstr,
         hashTypeCurrentFromZiplist(hi, what, vstr, vlen, vll);
     } else if (hi->encoding == OBJ_ENCODING_HT) {
         sds ele = hashTypeCurrentFromHashTable(hi, what);
-        *vstr = ele;
+        *vstr = (unsigned char*) ele;
         *vlen = sdslen(ele);
     } else {
         serverPanic("Unknown hash encoding");
@@ -424,14 +436,11 @@ void hashTypeConvertZiplist(robj *o, int enc) {
         dict = dictCreate(&hashDictType, NULL);
 
         while (hashTypeNext(hi) != C_ERR) {
-            unsigned char *vstr;
-            unsigned int vlen;
-            long long vll;
             sds key, value;
 
             key = hashTypeCurrentObjectNewSds(hi,OBJ_HASH_KEY);
             value = hashTypeCurrentObjectNewSds(hi,OBJ_HASH_VALUE);
-            ret = dictAdd(dict, field, value);
+            ret = dictAdd(dict, key, value);
             if (ret != DICT_OK) {
                 serverLogHexDump(LL_WARNING,"ziplist with dup elements dump",
                     o->ptr,ziplistBlobLen(o->ptr));
@@ -515,17 +524,17 @@ void hincrbyCommand(client *c) {
     robj *o;
     sds new;
     unsigned char *vstr;
-    unsigned int *lven;
+    unsigned int vlen;
 
     if (getLongLongFromObjectOrReply(c,c->argv[3],&incr,NULL) != C_OK) return;
     if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
-    if (hashTypeGetObject(o,c->argv[2],&vstr,&vlen,&value) == C_OK) {
+    if (hashTypeGetValue(o,c->argv[2]->ptr,&vstr,&vlen,&value) == C_OK) {
         if (vstr) {
-            if (string2ll(vstr,vlen,&value) == 0) {
+            if (string2ll((char*)vstr,vlen,&value) == 0) {
                 addReplyError(c,"hash value is not an integer");
                 return;
             }
-        } /* Else hashTypeGetObject() already stored it into &value */
+        } /* Else hashTypeGetValue() already stored it into &value */
     } else {
         value = 0;
     }
@@ -551,13 +560,13 @@ void hincrbyfloatCommand(client *c) {
     robj *o;
     sds new;
     unsigned char *vstr;
-    unsigned int *lven;
+    unsigned int vlen;
 
-    if (getLongDoubleFromObjectOrReply(c,c->argv[3],&incr,NULL) != C_OK) return;
+    if (getDoubleFromObjectOrReply(c,c->argv[3],&incr,NULL) != C_OK) return;
     if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
-    if (hashTypeGetObject(o,c->argv[2],vstr,vlen,&ll) == C_OK) {
+    if (hashTypeGetValue(o,c->argv[2]->ptr,&vstr,&vlen,&ll) == C_OK) {
         if (vstr) {
-            if (string2d(vstr,vlen,&value) == 0) {
+            if (string2d((char*)vstr,vlen,&value) == 0) {
                 addReplyError(c,"hash value is not an integer");
                 return;
             }
@@ -571,9 +580,9 @@ void hincrbyfloatCommand(client *c) {
     value += incr;
 
     char buf[256];
-    int len = ld2string(buf,sizeof(buf),value,humanfriendly);
+    int len = ld2string(buf,sizeof(buf),value,1);
     new = sdsnewlen(buf,len);
-    hashTypeSet(o,c->argv[2],new);
+    hashTypeSet(o,c->argv[2]->ptr,new);
     addReplyBulkSds(c,new);
     signalModifiedKey(c->db,c->argv[1]);
     notifyKeyspaceEvent(NOTIFY_HASH,"hincrbyfloat",c->argv[1],c->db->id);
@@ -587,12 +596,10 @@ void hincrbyfloatCommand(client *c) {
     newobj = createRawStringObject(buf,len);
     rewriteClientCommandArgument(c,0,aux);
     decrRefCount(aux);
-    rewriteClientCommandArgument(c,3,new);
+    rewriteClientCommandArgument(c,3,newobj);
     decrRefCount(newobj);
 }
 
-/* XXX From here. */
-
 static void addHashFieldToReply(client *c, robj *o, sds field) {
     int ret;
 
diff --git a/src/util.c b/src/util.c
index 80e316c9c..dba7f20f9 100644
--- a/src/util.c
+++ b/src/util.c
@@ -427,12 +427,17 @@ int string2l(const char *s, size_t slen, long *lval) {
  * a double: no spaces or other characters before or after the string
  * representing the number are accepted. */
 int string2d(const char *s, size_t slen, double *dp) {
+    char buf[256];
     double value;
+    char *eptr;
+
+    if (slen >= sizeof(buf)) return 0;
+    memcpy(buf,s,slen);
+    buf[slen] = '\0';
 
     errno = 0;
-    value = strtod(o->ptr, &eptr);
-    if (isspace(((char*)o->ptr)[0]) ||
-        eptr[0] != '\0' ||
+    value = strtod(buf, &eptr);
+    if (isspace(buf[0]) || eptr[0] != '\0' ||
         (errno == ERANGE &&
             (value == HUGE_VAL || value == -HUGE_VAL || value == 0)) ||
         errno == EINVAL ||
@@ -493,7 +498,6 @@ int d2string(char *buf, size_t len, double value) {
  * The function returns the length of the string or zero if there was not
  * enough buffer room to store it. */
 int ld2string(char *buf, size_t len, long double value, int humanfriendly) {
-    char buf[256];
     size_t l;
 
     if (isinf(value)) {