incr/decr doesn't check for -ve deltas, causing silent lossage #73

Closed
mbox opened this Issue Feb 14, 2012 · 1 comment

Projects

None yet

2 participants

@mbox
mbox commented Feb 14, 2012

Using pylibmc against memcached 1.4.5:

cache.set('wibble", 0)
>> 0
cache.decr('wibble', 1)
>> 0
cache.incr('wibble', -1)
>> 4294967295L
cache.incr('wibble', 1)
>> 4294967296L

This causes head-scratching lossage since there's no warnings or errors. The memcached protocol specifies that the Memcached will generate an error if a negative integer is passed using the ascii protocol - the problem is in pylibmc's conversion from Python to C.

The memcache command trace is:

<28 set :1:wibble 2 100000 1
> NOT FOUND :1:wibble
>28 STORED
<28 decr :1:wibble 1
> FOUND KEY :1:wibble
>28 0
<28 incr :1:wibble 4294967295
> FOUND KEY :1:wibble
>28 4294967295

Note that the -1 has been cast to an unsigned number, with resultant weird results.

The correct behaviour should be to error if passed a negative number. If it's felt that's an unexpected API change, then the incr()/decr() functions should detect a -ve number and call the inverse function with a +ve integer.

@lericson lericson added a commit that closed this issue Feb 20, 2012
@lericson Test for negative deltas in incr/decr
Fixes #73
60fa2d9
@lericson lericson closed this in 60fa2d9 Feb 20, 2012
@lericson
Owner

Four years later I realize 60fa2d9 was sufficient to fix #74 as well. Removing the so-called signedness check -- for how else would the two differ, unless the signed version is negative?

@lericson lericson added a commit that referenced this issue Mar 29, 2016
@lericson Remove useless overflow check
Refs #73, #74
5840a99
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment