[mb-devel] Possible Client SDK code bug

Andy Bridle andy at bridle.demon.co.uk
Wed Oct 25 08:55:26 UTC 2006


> OK, I see your point. But I would suggest this alternative:
> 
> m_bufferSize += ((size / kBufferSize) + 1) * kBufferSize;
> 
> This should always keep the m_BufferSize a multiple of 
> kBufferSize while keeping it large enough. If this wasn't my 
> goal originally, it should've been. :)
> 
> What do you think? Can you try the new code to see if it 
> crashes for you?
> 

I tried your suggested code, and my version of it, but I was still getting
occasional heap corruptions. A little further digging found another problem.

Imagine that the buffer is originally allocated with size = kBufferSize
(=8192) bytes. The function is then called 4 times with size = 2048 bytes,
filling the buffer. All is well until just before the function returns, when
it tries to set

	m_buffer[m_bytesInBuffer] = 0;

With 8192 bytes already in an 8192-byte buffer, this will write to memory
beyond the allocated size.

So, the buffer needs to be allocated (and re-allocated) with one extra byte
for the null char. I've amended my version of the code and, fingers crossed,
it now seems to be OK. The code in my version is as follows:

int MBHttp::WriteToBuffer(unsigned char *buffer, unsigned int size)
{
    if (m_buffer == NULL)
    {
        m_bufferSize = kBufferSize;
        //m_buffer = new unsigned char[m_bufferSize];
        // AB 20061025 above line changed to...
        m_buffer = new unsigned char[m_bufferSize + 1];
    }
    if (m_bytesInBuffer + size > m_bufferSize)
    {
        unsigned char *pTemp;

        //m_bufferSize += (kBufferSize < size) ? kBufferSize : kBufferSize +
size;
	  // AB 20061025 above line changed to...
	  m_bufferSize += ((size / kBufferSize) + 1) * kBufferSize;

	  //pTemp = new unsigned char[m_bufferSize];
	  // AB 20061025 above line changed to...
	  pTemp = new unsigned char[m_bufferSize + 1];

        memcpy(pTemp, m_buffer, m_bytesInBuffer);
        delete [] m_buffer;
        m_buffer = pTemp;
    }

    memcpy(m_buffer + m_bytesInBuffer, buffer, size);
    m_bytesInBuffer += size;
    m_buffer[m_bytesInBuffer] = 0;

    return size;
}

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




More information about the MusicBrainz-devel mailing list