diff --git a/src/streams.h b/src/streams.h --- a/src/streams.h +++ b/src/streams.h @@ -686,7 +686,6 @@ //! the buffer std::vector vchBuf; -protected: //! read data from the source to fill the buffer bool Fill() { unsigned int pos = nSrcPos % vchBuf.size(); @@ -708,6 +707,32 @@ return true; } + //! Advance the stream's read pointer (m_read_pos) by up to 'length' bytes, + //! filling the buffer from the file so that at least one byte is available. + //! Return a pointer to the available buffer data and the number of bytes + //! (which may be less than the requested length) that may be accessed + //! beginning at that pointer. + std::pair AdvanceStream(size_t length) { + assert(nReadPos <= nSrcPos); + if (nReadPos + length > nReadLimit) { + throw std::ios_base::failure( + "Attempt to position past buffer limit"); + } + // If there are no bytes available, read from the file. + if (nReadPos == nSrcPos && length > 0) { + Fill(); + } + + size_t buffer_offset{static_cast(nReadPos % vchBuf.size())}; + size_t buffer_available{ + static_cast(vchBuf.size() - buffer_offset)}; + size_t bytes_until_source_pos{static_cast(nSrcPos - nReadPos)}; + size_t advance{ + std::min({length, buffer_available, bytes_until_source_pos})}; + nReadPos += advance; + return std::make_pair(&vchBuf[buffer_offset], advance); + } + public: CBufferedFile(FILE *fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn) @@ -742,24 +767,19 @@ //! read a number of bytes void read(Span dst) { - if (dst.size() + nReadPos > nReadLimit) { - throw std::ios_base::failure("Read attempted past buffer limit"); - } while (dst.size() > 0) { - if (nReadPos == nSrcPos) { - Fill(); - } - unsigned int pos = nReadPos % vchBuf.size(); - size_t nNow = dst.size(); - if (nNow + pos > vchBuf.size()) { - nNow = vchBuf.size() - pos; - } - if (nNow + nReadPos > nSrcPos) { - nNow = nSrcPos - nReadPos; - } - memcpy(dst.data(), &vchBuf[pos], nNow); - nReadPos += nNow; - dst = dst.subspan(nNow); + auto [buffer_pointer, length]{AdvanceStream(dst.size())}; + memcpy(dst.data(), buffer_pointer, length); + dst = dst.subspan(length); + } + } + + //! Move the read position ahead in the stream to the given position. + //! Use SetPos() to back up in the stream, not SkipTo(). + void SkipTo(const uint64_t file_pos) { + assert(file_pos >= nReadPos); + while (nReadPos < file_pos) { + AdvanceStream(file_pos - nReadPos); } } diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -278,7 +278,7 @@ bf >> i; BOOST_CHECK(false); } catch (const std::exception &e) { - BOOST_CHECK(strstr(e.what(), "Read attempted past buffer limit") != + BOOST_CHECK(strstr(e.what(), "Attempt to position past buffer limit") != nullptr); } // The default argument removes the limit completely. @@ -348,7 +348,7 @@ BOOST_CHECK(!bf.SetPos(0)); // But we should now be positioned at least as far back as allowed // by the rewind window (relative to our farthest read position, 40). - BOOST_CHECK(bf.GetPos() <= 30); + BOOST_CHECK(bf.GetPos() <= 30U); // We can explicitly close the file, or the destructor will do it. bf.fclose(); @@ -356,6 +356,57 @@ fs::remove("streams_test_tmp"); } +BOOST_AUTO_TEST_CASE(streams_buffered_file_skip) { + fs::path streams_test_filename = + m_args.GetDataDirBase() / "streams_test_tmp"; + FILE *file = fsbridge::fopen(streams_test_filename, "w+b"); + // The value at each offset is the byte offset (e.g. byte 1 in the file has + // the value 0x01). + for (uint8_t j = 0; j < 40; ++j) { + fwrite(&j, 1, 1, file); + } + rewind(file); + + // The buffer is 25 bytes, allow rewinding 10 bytes. + CBufferedFile bf(file, 25, 10, 222, 333); + + uint8_t i; + // This is like bf >> (7-byte-variable), in that it will cause data + // to be read from the file into memory, but it's not copied to us. + bf.SkipTo(7); + BOOST_CHECK_EQUAL(bf.GetPos(), 7U); + bf >> i; + BOOST_CHECK_EQUAL(i, 7); + + // The bytes in the buffer up to offset 7 are valid and can be read. + BOOST_CHECK(bf.SetPos(0)); + bf >> i; + BOOST_CHECK_EQUAL(i, 0); + bf >> i; + BOOST_CHECK_EQUAL(i, 1); + + bf.SkipTo(11); + bf >> i; + BOOST_CHECK_EQUAL(i, 11); + + // SkipTo() honors the transfer limit; we can't position beyond the limit. + bf.SetLimit(13); + try { + bf.SkipTo(14); + BOOST_CHECK(false); + } catch (const std::exception &e) { + BOOST_CHECK(strstr(e.what(), "Attempt to position past buffer limit") != + nullptr); + } + + // We can position exactly to the transfer limit. + bf.SkipTo(13); + BOOST_CHECK_EQUAL(bf.GetPos(), 13U); + + bf.fclose(); + fs::remove(streams_test_filename); +} + BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) { // Make this test deterministic. SeedInsecureRand(SeedRand::ZEROS); @@ -386,7 +437,7 @@ // sizes; the boundaries of the objects can interact arbitrarily // with the CBufferFile's internal buffer. These first three // cases simulate objects of various sizes (1, 2, 5 bytes). - switch (InsecureRandRange(5)) { + switch (InsecureRandRange(6)) { case 0: { uint8_t a[1]; if (currentPos + 1 > fileSize) { @@ -427,6 +478,19 @@ break; } case 3: { + // SkipTo is similar to the "read" cases above, except + // we don't receive the data. + size_t skip_length{ + static_cast(InsecureRandRange(5))}; + if (currentPos + skip_length > fileSize) { + continue; + } + bf.SetLimit(currentPos + skip_length); + bf.SkipTo(currentPos + skip_length); + currentPos += skip_length; + break; + } + case 4: { // Find a byte value (that is at or ahead of the current // position). size_t find = currentPos + InsecureRandRange(8); @@ -445,7 +509,7 @@ currentPos++; break; } - case 4: { + case 5: { size_t requestPos = InsecureRandRange(maxPos + 4); bool okay = bf.SetPos(requestPos); // The new position may differ from the requested position