From: antirez <antirez@gmail.com>
Date: Thu, 24 Jun 2010 22:23:38 +0000 (+0200)
Subject: Merge remote branch 'pietern/zfixes'
X-Git-Url: https://git.saurik.com/redis.git/commitdiff_plain/b8b8501d70770465adc67d4effb4dbae2899f3a8?ds=inline;hp=-c

Merge remote branch 'pietern/zfixes'
---

b8b8501d70770465adc67d4effb4dbae2899f3a8
diff --combined redis.c
index 1e1cd781,085d3553..6fe951d3
--- a/redis.c
+++ b/redis.c
@@@ -5404,9 -5404,9 +5404,9 @@@ static void lrangeCommand(redisClient *
      if (start < 0) start = llen+start;
      if (end < 0) end = llen+end;
      if (start < 0) start = 0;
-     if (end < 0) end = 0;
  
-     /* indexes sanity checks */
+     /* Invariant: start >= 0, so this test will be true when end < 0.
+      * The range is empty when start > end or start >= length. */
      if (start > end || start >= llen) {
          /* Out of range start or start > end result in empty list */
          addReply(c,shared.emptymultibulk);
@@@ -5444,9 -5444,9 +5444,9 @@@ static void ltrimCommand(redisClient *c
      if (start < 0) start = llen+start;
      if (end < 0) end = llen+end;
      if (start < 0) start = 0;
-     if (end < 0) end = 0;
  
-     /* indexes sanity checks */
+     /* Invariant: start >= 0, so this test will be true when end < 0.
+      * The range is empty when start > end or start >= length. */
      if (start > end || start >= llen) {
          /* Out of range start or start > end result in empty list */
          ltrim = llen;
@@@ -6409,9 -6409,9 +6409,9 @@@ static void zremrangebyrankCommand(redi
      if (start < 0) start = llen+start;
      if (end < 0) end = llen+end;
      if (start < 0) start = 0;
-     if (end < 0) end = 0;
  
-     /* indexes sanity checks */
+     /* Invariant: start >= 0, so this test will be true when end < 0.
+      * The range is empty when start > end or start >= length. */
      if (start > end || start >= llen) {
          addReply(c,shared.czero);
          return;
@@@ -6662,11 -6662,10 +6662,10 @@@ static void zrangeGenericCommand(redisC
      if (start < 0) start = llen+start;
      if (end < 0) end = llen+end;
      if (start < 0) start = 0;
-     if (end < 0) end = 0;
  
-     /* indexes sanity checks */
+     /* Invariant: start >= 0, so this test will be true when end < 0.
+      * The range is empty when start > end or start >= length. */
      if (start > end || start >= llen) {
-         /* Out of range start or start > end result in empty list */
          addReply(c,shared.emptymultibulk);
          return;
      }
@@@ -8236,7 -8235,8 +8235,7 @@@ static void blockingPopGenericCommand(r
                  addReply(c,shared.wrongtypeerr);
                  return;
              } else {
 -                list *list = o->ptr;
 -                if (listLength(list) != 0) {
 +                if (listTypeLength(o) != 0) {
                      /* If the list contains elements fall back to the usual
                       * non-blocking POP operation */
                      robj *argv[2], **orig_argv;
diff --combined tests/unit/type/list.tcl
index 498cdfbb,8a7ea278..4636cc5b
--- a/tests/unit/type/list.tcl
+++ b/tests/unit/type/list.tcl
@@@ -78,99 -78,6 +78,99 @@@ start_server 
          assert_encoding linkedlist $key
      }
  
 +    foreach type {ziplist linkedlist} {
 +        test "BLPOP, BRPOP: single existing list - $type" {
 +            set rd [redis_deferring_client]
 +            create_$type blist {a b c d}
 +
 +            $rd blpop blist 1
 +            assert_equal {blist a} [$rd read]
 +            $rd brpop blist 1
 +            assert_equal {blist d} [$rd read]
 +
 +            $rd blpop blist 1
 +            assert_equal {blist b} [$rd read]
 +            $rd brpop blist 1
 +            assert_equal {blist c} [$rd read]
 +        }
 +
 +        test "BLPOP, BRPOP: multiple existing lists - $type" {
 +            set rd [redis_deferring_client]
 +            create_$type blist1 {a b c}
 +            create_$type blist2 {d e f}
 +
 +            $rd blpop blist1 blist2 1
 +            assert_equal {blist1 a} [$rd read]
 +            $rd brpop blist1 blist2 1
 +            assert_equal {blist1 c} [$rd read]
 +            assert_equal 1 [r llen blist1]
 +            assert_equal 3 [r llen blist2]
 +
 +            $rd blpop blist2 blist1 1
 +            assert_equal {blist2 d} [$rd read]
 +            $rd brpop blist2 blist1 1
 +            assert_equal {blist2 f} [$rd read]
 +            assert_equal 1 [r llen blist1]
 +            assert_equal 1 [r llen blist2]
 +        }
 +
 +        test "BLPOP, BRPOP: second list has an entry - $type" {
 +            set rd [redis_deferring_client]
 +            r del blist1
 +            create_$type blist2 {d e f}
 +
 +            $rd blpop blist1 blist2 1
 +            assert_equal {blist2 d} [$rd read]
 +            $rd brpop blist1 blist2 1
 +            assert_equal {blist2 f} [$rd read]
 +            assert_equal 0 [r llen blist1]
 +            assert_equal 1 [r llen blist2]
 +        }
 +    }
 +
 +    foreach {pop} {BLPOP BRPOP} {
 +        test "$pop: with single empty list argument" {
 +            set rd [redis_deferring_client]
 +            r del blist1
 +            $rd $pop blist1 1
 +            r rpush blist1 foo
 +            assert_equal {blist1 foo} [$rd read]
 +            assert_equal 0 [r exists blist1]
 +        }
 +
 +        test "$pop: second argument is not a list" {
 +            set rd [redis_deferring_client]
 +            r del blist1 blist2
 +            r set blist2 nolist
 +            $rd $pop blist1 blist2 1
 +            assert_error "ERR*wrong kind*" {$rd read}
 +        }
 +
 +        test "$pop: timeout" {
 +            set rd [redis_deferring_client]
 +            r del blist1 blist2
 +            $rd $pop blist1 blist2 1
 +            assert_equal {} [$rd read]
 +        }
 +
 +        test "$pop: arguments are empty" {
 +            set rd [redis_deferring_client]
 +            r del blist1 blist2
 +
 +            $rd $pop blist1 blist2 1
 +            r rpush blist1 foo
 +            assert_equal {blist1 foo} [$rd read]
 +            assert_equal 0 [r exists blist1]
 +            assert_equal 0 [r exists blist2]
 +
 +            $rd $pop blist1 blist2 1
 +            r rpush blist2 foo
 +            assert_equal {blist2 foo} [$rd read]
 +            assert_equal 0 [r exists blist1]
 +            assert_equal 0 [r exists blist2]
 +        }
 +    }
 +
      test {LPUSHX, RPUSHX - generic} {
          r del xlist
          assert_equal 0 [r lpushx xlist a]
@@@ -433,6 -340,12 +433,12 @@@
              create_$type mylist {1 2 3}
              assert_equal {1 2 3} [r lrange mylist -1000 1000]
          }
+ 
+         test "LRANGE out of range negative end index - $type" {
+             create_$type mylist {1 2 3}
+             assert_equal {1} [r lrange mylist 0 -3]
+             assert_equal {} [r lrange mylist 0 -4]
+         }
      }
  
      test {LRANGE against non existing key} {
@@@ -462,6 -375,11 +468,11 @@@
              assert_equal {1 2 3 4 5} [trim_list $type 0 10]
          }
  
+         test "LTRIM out of range negative end index - $type" {
+             assert_equal {1} [trim_list $type 0 -5]
+             assert_equal {} [trim_list $type 0 -6]
+         }
+ 
          tags {"slow"} {
              test "LTRIM stress testing - $type" {
                  set mylist {}