2
\$\begingroup\$

Initial review requested here:

The big thing I had missed was that read/write either work successfully (and all requested data is read/written) or it fails the flags are set appropriately.

For write this is easy. It work and we can return num or it fails and we return -1.

For read it is slightly more complex. A read that gets data but hits eof() has still worked (but my original function would have returned -1 indicating failure). So a read fails if the eof() is true before the read is tried or one of the following is true bad() or fail() or gcount() == 0 after the read is done.

/*
 * If we read and reach eof() that is still a good read (as long as data is read)
 * So this test does not check for eof() as a failure.
 *
 * Note: if eof() is true then the next call to streamReadThor()
 *       will result in a failure, as it test to see if the stream
 *       is good before a read.
 */
bool streamHadGoodRead(std::istream& stream)
{
    return !stream.bad() && !stream.fail() && stream.gcount() != 0;
}

/*
 * Reads `num` objects of size `size` from the stream.
 * Return -1 on failure or the number of objects read.
 */
size_t streamReadThor(SDL_RWops* input, void* dst, size_t size, size_t num)
{
    ThorSDLStreamRead*  data = static_cast<ThorSDLStreamRead*>(input);
    if (!data->stream) {
        return -1;
    }
    data->stream.read(reinterpret_cast<char*>(dst), num * size);
    return streamHadGoodRead(data->stream) ? data->stream.gcount() / size : -1;
}

/*
 * Write `num` objects of size `size` to the stream.
 * Return -1 on failure or the number of objects written.
 */
size_t streamWriteThor(SDL_RWops* output, const void* src, size_t size, size_t num)
{
    ThorSDLStreamWrite*  data = static_cast<ThorSDLStreamWrite*>(output);
    if (!data->stream) {
        return -1;
    }
    data->stream.write(reinterpret_cast<char const*>(src), num * size);
    return data->stream ? num : -1;
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Make sure the comments match the code. In streamHadGoodRead() the comments talk about eof(), but the code doesn't contain any eof().

Furthermore, the code is still not correct. Read the documentation of std::ifstream::read() carefully: you'll notice that on reaching the end of the file, it will set both eofbit and failbit.

I also checked the documentation for SDL_RWops: apparently, read() should return 0 on error or EOF, not -1. That also means that you don't need to special case for gcount() == 0. Also, write() should return the number of objects that were fully written, and not -1 on error.

/*
 * Read `num` objects of size `size` from the stream.
 * Return the number of objects successfully read,
 * which might be less than num if an error occurred or EOF has been reached.
 */
size_t streamReadThor(SDL_RWops* input, void* dst, size_t size, size_t num)
{
    auto& stream = static_cast<ThorSDLStreamRead*>(input)->stream;
    stream.read(reinterpret_cast<char*>(dst), num * size);
    return stream.gcount() / size;
}

/*
 * Write `num` objects of size `size` to the stream.
 * Return the number of objects successfully written,
 * which might be less than num if an error occurred.
 */
size_t streamWriteThor(SDL_RWops* output, const void* src, size_t size, size_t num)
{
    auto& stream = static_cast<ThorSDLStreamWrite*>(output)->stream;

    if (!stream)
        return 0;

    auto before = stream.tellp();
    stream.write(reinterpret_cast<char const*>(src), num * size);
    return (stream.tellp() - before) / size;
}

Note that std::ostream::write() does not set failbit on error, so tellp() would still be valid after a write error. However, if failbit was set because of any other action on the stream, then tellp() cannot be used, so we still need the check if the stream is good before doing anything.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.