Bug in BufferedInputStream

General JUCE discussion
1

If the source stream for some reason fails, ensureBuffered will never exit.

BufferedInputStream.h

private:
    InputStream* const source;
    const bool deleteSourceWhenDestroyed;
    int bufferSize;
    int64 position, lastReadPos, bufferStart, bufferOverlap;
    char* buffer;
    bool ensureBuffered();  // Returns bool now

    BufferedInputStream (const BufferedInputStream&);
    const BufferedInputStream& operator= (const BufferedInputStream&);
};

BufferedInputStream::ensureBuffered() :

        else
        {
            bufferStart = position;
            source->setPosition (bufferStart);
            bytesRead = source->read (buffer, bufferSize);
            if (bytesRead < jmin((int64)bufferSize, source->getTotalLength() - bufferStart))
            {
                // We've read too little
                return false;
            }
            lastReadPos = bufferStart + bytesRead;
        }

and BufferedInputStream::read(…) :

int BufferedInputStream::read (void* destBuffer, int maxBytesToRead)
{
    if (position >= bufferStart
         && position + maxBytesToRead <= lastReadPos)
    {
        memcpy (destBuffer, buffer + (position - bufferStart), maxBytesToRead);
        position += maxBytesToRead;

        return maxBytesToRead;
    }
    else
    {
        if (position < bufferStart || position >= lastReadPos)
        {
            if (!ensureBuffered())
            {
                return 0;
            }
        }

        int bytesRead = 0;

        while (maxBytesToRead > 0)
        {
            const int bytesAvailable = jmin (maxBytesToRead, (int) (lastReadPos - position));

            if (bytesAvailable > 0)
            {
                memcpy (destBuffer, buffer + (position - bufferStart), bytesAvailable);
                maxBytesToRead -= bytesAvailable;
                bytesRead += bytesAvailable;
                position += bytesAvailable;
                destBuffer = (void*) (((char*) destBuffer) + bytesAvailable);
            }

            if (!ensureBuffered())
                break;

            if (isExhausted())
                break;
        }

        return bytesRead;
    }
}

which will let the application fail gracefully.

2

Thanks - that’s a good bug, but not sure if your fix is quite right… It looks to me like if the source stream fails, your solution could lose the last bytes of data from it, which might not be what you want. It also doesn’t look like it’d work with streams that don’t know their own total length.

How about this slightly simpler idea, at line 156 of the original code?

[code] const int64 oldLastReadPos = lastReadPos;
ensureBuffered();

        if (oldLastReadPos == lastReadPos)
            break; // if ensureBuffered() failed to read any more data, bail out

        if (isExhausted())
            break;
    }[/code]
3

The simpler the better, works for me!