Incorrect Reallocation Size for Stats (ver 1.3.3)

[GoogleCodeExporter]

Hi!

The reallocation size in grow_stats_buf() in memcached.c is incorrect, thus 
causes a 
segmentation fault when we encounter a large stats output (e.g. when the daemon 
has over 40 
slab classes).

We only reallocate the number of bytes that will accommodate the "needed" size 
plus a little 
more from doubling the nsize variable. The problem is that we realloc() based 
on this size. What 
we need to do is add the offset size to guarantee enough size: 

  realloc(c->stats.buffer, nsize + c->stats.offset);

Here's the patch on github:

http://github.com/tmaesaka/memcached/commit/404fd37417a6758d468d8c9f70e4c643c2b8
9
051

Cheers,
Toru

Original issue reported on code.google.com by tmaesaka on 6 Apr 2009 at 6:05

[GoogleCodeExporter]

Hey, I put a comment up on github, but I think keeping the allocations as 
powers of 2
is OK, but we need to ensure the power of two is large enough to hold what we're
going to put into it.

That is, I think the while loop is wrong, not the realloc.

Original comment by dsalli...@gmail.com on 6 Apr 2009 at 6:29

[GoogleCodeExporter]

Hi!

Thanks for a quick feedback. Either way would work fine but you've got a point, 
sticking to the powers of two 
approach sounds more sensible in this case. Lemmie quickly whip up another 
patch and this time I'll also write a 
test for it.

I'll throw another review request soon :)

Original comment by tmaesaka on 6 Apr 2009 at 6:51

[GoogleCodeExporter]

Here's a patch that fixes the while loop as you mentioned. Yeah, it made more 
sense to fix the loop due to the 
if block that comes after the loop.

http://github.com/tmaesaka/memcached/commit/e97ee810c0ee183f0a374d34e2dececdc252
77c8

 I've also added a test case for this bug :)

http://github.com/tmaesaka/memcached/blob/e97ee810c0ee183f0a374d34e2dececdc25277
c8/t/issue_41.t

Cheers,
Toru

Original comment by tmaesaka on 6 Apr 2009 at 8:10

[GoogleCodeExporter]

Thanks so much for the test.  That made a huge difference.

I want to refactor this growth thing a bit just to make it clearer.  The fact 
that
it's taking so long to get this right is a bad sign, it seems.

Original comment by dsalli...@gmail.com on 6 Apr 2009 at 5:45

• Changed state: Fixed