[PATCH] Bugfix for incorrect length when decr a value

[caokai]

protocol.txt is the "absolute truth"?

I would prefer that vlen to be 4 in "9999",not 5 in "9999 ", if there is no big performance degradation.

the protocol.txt:Note also that decrementing a number such that it loses length isn't
guaranteed to decrement its returned length.  The number MAY be
space-padded at the end, but this is purely an implementation
optimization, so you also shouldn't rely on that.----- Original Message ----- 
From: "Dustin Sallings" <dustin at spy.net>
To: "Steve Chu" <stvchu at gmail.com>
Cc: "memcached list" <memcached at lists.danga.com>
Sent: Friday, October 19, 2007 12:19 AM
Subject: Re: [PATCH] Bugfix for incorrect length when decr a value


> 
>   This is documented in protocol.txt.
> 
> --  
> Dustin Sallings (mobile)
> 
> On Oct 18, 2007, at 3:38, "Steve Chu" <stvchu at gmail.com> wrote:
> 
>> Sorry, all.
>> I made a mistake, the vlen *is* 5, because 9999 with a suffix of
>> blank, but that is really weird. libmemcache is  ok for this.
>>
>> On 10/18/07, Steve Chu <stvchu at gmail.com> wrote:
>>> Anybody can have a try:
>>> -----------------------------------------------
>>> # telnet 127.0.0.1 11211
>>> Trying 127.0.0.1...
>>> Connected to 127.0.0.1 (127.0.0.1).
>>> Escape character is '^]'.
>>> get test
>>> END
>>> add test 0 0 4
>>> 9999
>>> STORED
>>> incr test 1
>>> 10000
>>> get test
>>> VALUE test 0 5
>>> 10000
>>> END
>>> decr test 1
>>> 9999
>>> get test
>>> VALUE test 0 5
>>> 9999
>>> END
>>> --------------
>>> The vlen does not shrink when *decr* from 10000 to 9999, it is  
>>> still 5!!
>>> This bug does not affect most dynamic language client(php .etc), but
>>> for clients that
>>> use this vlen, they don't work, for example, libmemcache.
>>> Also it does not follow the memcache protocol.
>>> Here comes the pacth:
>>> ===========================================================
>>> --- memcached-trunk/memcached.c 2007-10-18 15:57:28.000000000 +0800
>>> +++ memcached-new/memcached.c   2007-10-18 15:54:49.000000000 +0800
>>> @@ -1366,6 +1366,7 @@ char *do_add_delta(item *it, const bool
>>>     char *ptr;
>>>     int64_t value;
>>>     int res;
>>> +    item *new_it;
>>>
>>>     ptr = ITEM_data(it);
>>>     while ((*ptr != '\0') && (*ptr < '0' && *ptr > '9')) ptr++;    //
>>> BUG: can't be true
>>> @@ -1384,20 +1385,17 @@ char *do_add_delta(item *it, const bool
>>>     }
>>>     sprintf(buf, "%llu", value);
>>>     res = strlen(buf);
>>> -    if (res + 2 > it->nbytes) { /* need to realloc */
>>> -        item *new_it;
>>> -        new_it = do_item_alloc(ITEM_key(it), it->nkey,
>>> atoi(ITEM_suffix(it) + 1), it->exptime, res + 2 );
>>> -        if (new_it == 0) {
>>> -            return "SERVER_ERROR out of memory";
>>> -        }
>>> -        memcpy(ITEM_data(new_it), buf, res);
>>> -        memcpy(ITEM_data(new_it) + res, "\r\n", 3);
>>> -        do_item_replace(it, new_it);
>>> -        do_item_remove(new_it);       /* release our reference */
>>> -    } else { /* replace in-place */
>>> -        memcpy(ITEM_data(it), buf, res);
>>> -        memset(ITEM_data(it) + res, ' ', it->nbytes - res - 2);
>>> +
>>> +    /* just alloc one, not plan to new one only when res + 2 > it- 
>>> >nbytes */
>>> +    new_it = do_item_alloc(ITEM_key(it), it->nkey,
>>> atoi(ITEM_suffix(it) + 1), it->exptime, res + 2 );
>>> +
>>> +    if (new_it == 0) {
>>> +        return "SERVER_ERROR out of memory";
>>>     }
>>> +    memcpy(ITEM_data(new_it), buf, res);
>>> +    memcpy(ITEM_data(new_it) + res, "\r\n", 3);
>>> +    do_item_replace(it, new_it);
>>> +    do_item_remove(new_it);       /* release our reference */
>>>
>>>     return buf;
>>> }
>>> ========================================================
>>>
>>> --
>>> Steve Chu
>>> http://www.sina.com
>>>
>>
>>
>> -- 
>> Steve Chu
>> http://stvchu.org
>>
>>
>