Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

BinaryConverter.DecodeUInt64 error #77

Closed
slegay opened this Issue Aug 14, 2011 · 1 comment

Comments

Projects
None yet
2 participants

slegay commented Aug 14, 2011

We were using the incr/decr feature to track bandwidth usage and noticed invalid values when working outside the UInt32 range. It seems like there's an error in the binary converter when working with 8-byte integers.

Here's a test:

        [TestCase]
        public void IncrementLongTest()
        {
            var intialValue = 56UL * (ulong)System.Math.Pow(10, 11) + 1234;
            using (MemcachedClient client = GetClient())
            {
                Assert.AreEqual(intialValue, client.Increment("VALUE", intialValue, 2UL), "Non-exsiting value should be set to default");
                Assert.AreEqual(intialValue + 24, client.Increment("VALUE", 10UL, 24UL));
            }
        }

... and here's a fix using a reversed array approach rather than bit shifting (no idea if that's efficient, but it seems to work - all tests passed). In Enyim.Caching.Memcached.Protocol.Binary.BinaryConverter, line 51:

public static unsafe ulong DecodeUInt64(byte* buffer, int offset)
{

            buffer += offset;

            byte[] copy = new byte[8] { 
                buffer[7],
                buffer[6],
                buffer[5],
                buffer[4],
                buffer[3],
                buffer[2],
                buffer[1],
                buffer[0]
            };
            return BitConverter.ToUInt64(copy, 0);
}

instead of:

Owner

enyim commented Oct 3, 2011

Thanks.

The bug was with the long/ulong casting, so the upper part of the counter was 0xfffffsomething instead of 0x000something

@enyim enyim closed this Oct 3, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment