Skip to content

Commit

Permalink
Correctly handle failing mmap on FileReader.
Browse files Browse the repository at this point in the history
If we try to mmap a region to high (>4GB), `makeMmappedBuffer` will
throw a `MMapException`. In this case, we want to do a read in the file
instead of mmap.

This case is handled in `MultiPartFileReader` but was missing in
`FileReader`.

Fix #866
  • Loading branch information
mgautierfr committed Mar 22, 2024
1 parent f624344 commit 13b94f2
Showing 1 changed file with 19 additions and 8 deletions.
27 changes: 19 additions & 8 deletions src/file_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,10 @@ const Buffer MultiPartFileReader::get_buffer(offset_t offset, zsize_t size) cons
} catch(MMapException& e)
#endif
{
// The range is several part, or we are on Windows.
// We cannot do the mmap, for several possible reasons:
// - Mmap offset is too big (>4GB on 32 bits)
// - The range is several part
// - We are on Windows.
// We will have to do some memory copies :/
// [TODO] Use Windows equivalent for mmap.
auto ret_buffer = Buffer::makeBuffer(size);
Expand Down Expand Up @@ -278,14 +281,22 @@ const Buffer FileReader::get_buffer(offset_t offset, zsize_t size) const
{
ASSERT(size, <=, _size);
#ifdef ENABLE_USE_MMAP
offset += _offset;
int fd = _fhandle->getNativeHandle();
return Buffer::makeBuffer(makeMmappedBuffer(fd, offset, size), size);
#else // We are on Windows. [TODO] Use Windows equivalent for mmap.
auto ret_buffer = Buffer::makeBuffer(size);
read(const_cast<char*>(ret_buffer.data()), offset, size);
return ret_buffer;
try {
auto local_offset = offset + _offset;
int fd = _fhandle->getNativeHandle();

Check warning on line 286 in src/file_reader.cpp

View check run for this annotation

Codecov / codecov/patch

src/file_reader.cpp#L286

Added line #L286 was not covered by tests
return Buffer::makeBuffer(makeMmappedBuffer(fd, local_offset, size), size);
} catch(MMapException& e)
#endif
{
// We cannot do the mmap, for several possible reasons:
// - Mmap offset is too big (>4GB on 32 bits)
// - We are on Windows.
// We will have to do some memory copies :/
// [TODO] Use Windows equivalent for mmap.
auto ret_buffer = Buffer::makeBuffer(size);

Check warning on line 296 in src/file_reader.cpp

View check run for this annotation

Codecov / codecov/patch

src/file_reader.cpp#L296

Added line #L296 was not covered by tests
read(const_cast<char*>(ret_buffer.data()), offset, size);
return ret_buffer;
}
}

std::unique_ptr<const Reader>
Expand Down

0 comments on commit 13b94f2

Please sign in to comment.