From: antirez <antirez@gmail.com>
Date: Tue, 31 Aug 2010 09:23:12 +0000 (+0200)
Subject: resolved conflict merging pietern/bpop-timeout
X-Git-Url: https://git.saurik.com/redis.git/commitdiff_plain/f7f12a606c39fcb09a203faaaa12c49882409d8f?ds=sidebyside;hp=-c

resolved conflict merging pietern/bpop-timeout
---

f7f12a606c39fcb09a203faaaa12c49882409d8f
diff --combined src/t_list.c
index 6b4a611e,43d292b6..add1bee1
--- a/src/t_list.c
+++ b/src/t_list.c
@@@ -782,9 -782,20 +782,20 @@@ int handleClientsWaitingListPush(redisC
  /* Blocking RPOP/LPOP */
  void blockingPopGenericCommand(redisClient *c, int where) {
      robj *o;
+     long long lltimeout;
      time_t timeout;
      int j;
  
+     /* Make sure timeout is an integer value */
+     if (getLongLongFromObjectOrReply(c,c->argv[c->argc-1],&lltimeout,
+             "timeout is not an integer") != REDIS_OK) return;
+ 
+     /* Make sure the timeout is not negative */
+     if (lltimeout < 0) {
+         addReplySds(c,sdsnew("-ERR timeout is negative\r\n"));
+         return;
+     }
+ 
      for (j = 1; j < c->argc-1; j++) {
          o = lookupKeyWrite(c->db,c->argv[j]);
          if (o != NULL) {
@@@ -823,15 -834,9 +834,16 @@@
              }
          }
      }
+ 
 +    /* If we are inside a MULTI/EXEC and the list is empty the only thing
 +     * we can do is treating it as a timeout (even with timeout 0). */
 +    if (c->flags & REDIS_MULTI) {
 +        addReply(c,shared.nullmultibulk);
 +        return;
 +    }
 +
      /* If the list is empty or the key does not exists we must block */
-     timeout = strtol(c->argv[c->argc-1]->ptr,NULL,10);
+     timeout = lltimeout;
      if (timeout > 0) timeout += time(NULL);
      blockForKeys(c,c->argv+1,c->argc-2,timeout);
  }
diff --combined tests/unit/type/list.tcl
index ca0da764,1a85c809..bf188fd7
--- a/tests/unit/type/list.tcl
+++ b/tests/unit/type/list.tcl
@@@ -139,6 -139,28 +139,28 @@@ start_server 
              assert_equal 0 [r exists blist1]
          }
  
+         test "$pop: with negative timeout" {
+             set rd [redis_deferring_client]
+             $rd $pop blist1 -1
+             assert_error "ERR*is negative*" {$rd read}
+         }
+ 
+         test "$pop: with non-integer timeout" {
+             set rd [redis_deferring_client]
+             $rd $pop blist1 1.1
+             assert_error "ERR*not an integer*" {$rd read}
+         }
+ 
+         test "$pop: with zero timeout should block indefinitely" {
+             # To test this, use a timeout of 0 and wait a second.
+             # The blocking pop should still be waiting for a push.
+             set rd [redis_deferring_client]
+             $rd $pop blist1 0
+             after 1000
+             r rpush blist1 foo
+             assert_equal {blist1 foo} [$rd read]
+         }
+ 
          test "$pop: second argument is not a list" {
              set rd [redis_deferring_client]
              r del blist1 blist2
@@@ -172,17 -194,6 +194,17 @@@
          }
      }
  
 +    test {BLPOP inside a transaction} {
 +        r del xlist
 +        r lpush xlist foo
 +        r lpush xlist bar
 +        r multi
 +        r blpop xlist 0
 +        r blpop xlist 0
 +        r blpop xlist 0
 +        r exec
 +    } {{xlist bar} {xlist foo} {}}
 +
      test {LPUSHX, RPUSHX - generic} {
          r del xlist
          assert_equal 0 [r lpushx xlist a]