CAS is broken

Tomash Brechko tomash.brechko at gmail.com
Thu Nov 15 09:55:14 UTC 2007


On Thu, Nov 15, 2007 at 00:56:18 -0800, dormando wrote:
> For posterity, fix is in r635 from chris (not christ!) and dustin.
> I read it, played with it a bit, ran tests, ran it, looked okay.
> Runs entirely inside a lock, logic appears sane.
> 
> Adds 8 bytes to the item struct.

Thanks for the good patch!  There are two minor comments below:


> --- a/trunk/server/items.c
> +++ b/trunk/server/items.c
> @@ -39,6 +40,15 @@ void item_init(void) {
>      }
>  }
>  
> +/* Get the next CAS id for a new item. */
> +uint64_t get_cas_id() {
> +    static uint64_t cas_id;
> +    if(cas_id >= MAX_CAS_ID) {
> +        cas_id = 0;
> +    }
> +    return ++cas_id;
> +}
> +

This test against MAX_CAS_ID is not needed: C standard _guarantees_
that ++ for unsigned type will wraparound to zero automagically.  And
this condition won't be ever met anyway, so the check is a bit of a
waste (unless you have some reasons not to use full 64 bit range that
I'm failing to see, of course).


> @@ -697,11 +698,15 @@ static void complete_nread(conn *c) {
...
> +      else if(ret == 3)
> +          out_string(c, "NOT FOUND");

Should be "NOT_FOUND" (with underscore).  This typo was present in the
original code, and was accidentally copied here.


-- 
   Tomash Brechko


More information about the memcached mailing list