[mb-devel] Possible Client SDK code bug

[Andy Bridle]

> -----Original Message-----
> From: musicbrainz-devel-bounces at lists.musicbrainz.org 
> [mailto:musicbrainz-devel-bounces at lists.musicbrainz.org] On 
> Behalf Of Robert Kaye
> Sent: 23 October 2006 22:26
> To: MusicBrainz development list
> Subject: Re: [mb-devel] Possible Client SDK code bug
> 
> 
> On Oct 21, 2006, at 4:47 AM, Andy Bridle wrote:
> >
> > The source for member function MBHttp::WriteToBuffer contains the 
> > following code to calculate the new size for the dynamic buffer:
> >
> >     if (m_bytesInBuffer + size > m_bufferSize)
> >     {
> > 	  .
> >         m_bufferSize += (kBufferSize < size) ? kBufferSize :  
> > kBufferSize +
> > size;
> >         .
> >     }
> >
> > Which, to me, looks the wrong way round, ie it should be:
> >
> >         m_bufferSize += (kBufferSize < size) ? kBufferSize + size :
> > kBufferSize;
> >
> > But please feel free to make me look silly.
> 
> Did you see that the line in question is INCREASING the 
> allocated buffer size? Its not calculating a total new size. 
> The code looks OK to me.
> 

Well, it still looks wrong to me. I know it's increasing the buffer size,
but it's not increasing it enough. If (kBufferSize < size) it only increases
the buffer size by kBufferSize, and then appends 'size' characters to it.
This corrupts the heap. This isn't picked up by the memcpy (hence memcpy now
being deprecated by MS), but it is detected when the 'delete [] m_buffer' is
done next time through.

I'm not feeling silly yet.

-- 
No virus found in this outgoing message.
Checked by AVG Free Edition.
Version: 7.1.408 / Virus Database: 268.13.10/491 - Release Date: 23/10/2006