Fixed compareStringObject() and introduced collateStringObject().

compareStringObject was not always giving the same result when comparing
two exact strings, but encoded as integers or as sds strings, since it
switched to strcmp() when at least one of the strings were not sds
encoded.

For instance the two strings "123" and "123\x00456", where the first
string was integer encoded, would result into the old implementation of
compareStringObject() to return 0 as if the strings were equal, while
instead the second string is "greater" than the first in a binary
comparison.

The same compasion, but with "123" encoded as sds string, would instead
return a value < 0, as it is correct. It is not impossible that the
above caused some obscure bug, since the comparison was not always
deterministic, and compareStringObject() is used in the implementation
of skiplists, hash tables, and so forth.

At the same time, collateStringObject() was introduced by this commit, so
that can be used by SORT command to return sorted strings usign
collation instead of binary comparison. See next commit.

2 files changed, 34 insertions, 8 deletions

diff --git a/src/object.c b/src/object.c
index 2554656a3..58668da5b 100644
--- a/src/object.c
+++ b/src/object.c
@@ -332,35 +332,60 @@ robj *getDecodedObject(robj *o) {
     }
 }
 
-/* Compare two string objects via strcmp() or alike.
+/* Compare two string objects via strcmp() or strcoll() depending on flags.
  * Note that the objects may be integer-encoded. In such a case we
  * use ll2string() to get a string representation of the numbers on the stack
  * and compare the strings, it's much faster than calling getDecodedObject().
  *
- * Important note: if objects are not integer encoded, but binary-safe strings,
- * sdscmp() from sds.c will apply memcmp() so this function ca be considered
- * binary safe. */
-int compareStringObjects(robj *a, robj *b) {
+ * Important note: when REDIS_COMPARE_BINARY is used a binary-safe comparison
+ * is used. */
+
+#define REDIS_COMPARE_BINARY (1<<0)
+#define REDIS_COMPARE_COLL (1<<1)
+
+int compareStringObjectsWithFlags(robj *a, robj *b, int flags) {
     redisAssertWithInfo(NULL,a,a->type == REDIS_STRING && b->type == REDIS_STRING);
     char bufa[128], bufb[128], *astr, *bstr;
+    size_t alen, blen, minlen;
     int bothsds = 1;
 
     if (a == b) return 0;
     if (a->encoding != REDIS_ENCODING_RAW) {
-        ll2string(bufa,sizeof(bufa),(long) a->ptr);
+        alen = ll2string(bufa,sizeof(bufa),(long) a->ptr);
         astr = bufa;
         bothsds = 0;
     } else {
         astr = a->ptr;
+        alen = sdslen(astr);
     }
     if (b->encoding != REDIS_ENCODING_RAW) {
-        ll2string(bufb,sizeof(bufb),(long) b->ptr);
+        blen = ll2string(bufb,sizeof(bufb),(long) b->ptr);
         bstr = bufb;
         bothsds = 0;
     } else {
         bstr = b->ptr;
+        blen = sdslen(bstr);
+    }
+    if (flags & REDIS_COMPARE_COLL) {
+        return strcoll(astr,bstr);
+    } else {
+        int cmp;
+
+        minlen = (alen < blen) ? alen : blen;
+        cmp = memcmp(astr,bstr,minlen);
+        if (cmp == 0) return alen-blen;
+        return cmp;
     }
-    return bothsds ? sdscmp(astr,bstr) : strcmp(astr,bstr);
+}
+
+/* Wrapper for compareStringObjectsWithFlags() using binary comparison. */
+int compareStringObjects(robj *a, robj *b) {
+    return compareStringObjectsWithFlags(a,b,REDIS_COMPARE_BINARY);
+}
+
+/* Wrapper for compareStringObjectsWithFlags() using collation. */
+int collateStringObjects(robj *a, robj *b) {
+    return compareStringObjectsWithFlags(a,b,REDIS_COMPARE_COLL);
 }
 
 /* Equal string objects return 1 if the two objects are the same from the
diff --git a/src/redis.h b/src/redis.h
index f3ef3fbfd..0fb8e3028 100644
--- a/src/redis.h
+++ b/src/redis.h
@@ -1159,6 +1159,7 @@ int getLongDoubleFromObject(robj *o, long double *target);
 int getLongDoubleFromObjectOrReply(redisClient *c, robj *o, long double *target, const char *msg);
 char *strEncoding(int encoding);
 int compareStringObjects(robj *a, robj *b);
+int collateStringObjects(robj *a, robj *b);
 int equalStringObjects(robj *a, robj *b);
 unsigned long estimateObjectIdleTime(robj *o);