From dc885d0984d4c2d0e8029b898aeb68a10a455be5 Mon Sep 17 00:00:00 2001 From: mandreyel Date: Mon, 27 May 2019 18:10:43 +0200 Subject: [PATCH 1/3] Add tests for mappings with different file offsets --- test/test.cpp | 127 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 51 deletions(-) diff --git a/test/test.cpp b/test/test.cpp index b0a9c24..82827f9 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -15,29 +15,32 @@ #include #endif -int handle_error(const std::error_code& error) -{ - const auto& errmsg = error.message(); - std::printf("error mapping file: %s, exiting...\n", errmsg.c_str()); - return error.value(); -} - // Just make sure this compiles. #ifdef CXX17 # include using mmap_source = mio::basic_mmap_source; #endif +template +void test_at_offset(const MMap& file_view, const std::string& buffer, + const size_t offset); +void test_at_offset(const std::string& buffer, const char* path, + const size_t offset, std::error_code& error); +int handle_error(const std::error_code& error); + int main() { - const char _path[] = "test-file"; + std::error_code error; + // Make sure mio compiles with non-const char* strings too. + const char _path[] = "test-file"; const int path_len = sizeof(_path); char* path = new char[path_len]; std::copy(_path, _path + path_len, path); - std::error_code error; + + const auto page_size = mio::page_size(); // Fill buffer, then write it to file. - const int file_size = 0x4000 - 250; // 16134 + const int file_size = 4 * page_size - 250; // 16134, if page size is 4KiB std::string buffer(file_size, 0); // Start at first printable ASCII character. char v = 33; @@ -50,54 +53,26 @@ int main() v = 33; } } + std::ofstream file(path); file << buffer; file.close(); - const size_t offset = 300; - assert(offset < buffer.size()); + // Test whole file mapping. + test_at_offset(buffer, path, 0, error); + if (error) { return handle_error(error); } - { - // Map the region of the file to which buffer was written. - mio::mmap_source file_view = mio::make_mmap_source( - path, offset, mio::map_entire_file, error); - if(error) { return handle_error(error); } + // Test starting from below the page size. + test_at_offset(buffer, path, page_size - 3, error); + if (error) { return handle_error(error); } - const size_t mapped_size = buffer.size() - offset; // 15834 - assert(file_view.is_open()); - assert(file_view.size() == mapped_size); + // Test starting from above the page size. + test_at_offset(buffer, path, page_size + 3, error); + if (error) { return handle_error(error); } - // Then verify that mmap's bytes correspond to that of buffer. - for(size_t buf_idx = offset, view_idx = 0; - buf_idx < buffer.size() && view_idx < mapped_size; - ++buf_idx, ++view_idx) { - if(file_view[view_idx] != buffer[buf_idx]) { - std::printf("%luth byte mismatch: expected(%d) <> actual(%d)", - buf_idx, buffer[buf_idx], file_view[view_idx]); - std::cout << std::flush; - assert(0); - } - } - - // Turn file_view into a shared mmap. - mio::shared_mmap_source shared_file_view(std::move(file_view)); - - assert(!file_view.is_open()); - assert(shared_file_view.is_open()); - assert(shared_file_view.size() == mapped_size); - - // Then verify that shared_mmap's bytes correspond to that of buffer. - for(size_t buf_idx = offset, view_idx = 0; - buf_idx < buffer.size() && view_idx < mapped_size; - ++buf_idx, ++view_idx) { - if(shared_file_view[view_idx] != buffer[buf_idx]) { - std::printf("%luth byte mismatch: expected(%d) <> actual(%d)", - buf_idx, buffer[buf_idx], shared_file_view[view_idx]); - std::cout << std::flush; - assert(0); - } - } - } + // Test starting from above the page size. + test_at_offset(buffer, path, 2 * page_size + 3, error); + if (error) { return handle_error(error); } { #define CHECK_INVALID_MMAP(m) do { \ @@ -155,3 +130,53 @@ int main() std::printf("all tests passed!\n"); } + +void test_at_offset(const std::string& buffer, const char* path, + const size_t offset, std::error_code& error) +{ + // Sanity check. + assert(offset < buffer.size()); + + // Map the region of the file to which buffer was written. + mio::mmap_source file_view = mio::make_mmap_source( + path, offset, mio::map_entire_file, error); + if(error) { return; } + + assert(file_view.is_open()); + const size_t mapped_size = buffer.size() - offset; + assert(file_view.size() == mapped_size); + + test_at_offset(file_view, buffer, offset); + + // Turn file_view into a shared mmap. + mio::shared_mmap_source shared_file_view(std::move(file_view)); + assert(!file_view.is_open()); + assert(shared_file_view.is_open()); + assert(shared_file_view.size() == mapped_size); + + //test_at_offset(shared_file_view, buffer, offset); +} + +template +void test_at_offset(const MMap& file_view, const std::string& buffer, + const size_t offset) +{ + // Then verify that mmap's bytes correspond to that of buffer. + for(size_t buf_idx = offset, view_idx = 0; + buf_idx < buffer.size() && view_idx < file_view.size(); + ++buf_idx, ++view_idx) { + if(file_view[view_idx] != buffer[buf_idx]) { + std::printf("%luth byte mismatch: expected(%d) <> actual(%d)", + buf_idx, buffer[buf_idx], file_view[view_idx]); + std::cout << std::flush; + assert(0); + } + } +} + +int handle_error(const std::error_code& error) +{ + const auto& errmsg = error.message(); + std::printf("Error mapping file: %s, exiting...\n", errmsg.c_str()); + return error.value(); +} From 518b99b9672c28c94c8caa5f8007c60faebcc847 Mon Sep 17 00:00:00 2001 From: mandreyel Date: Mon, 27 May 2019 18:38:27 +0200 Subject: [PATCH 2/3] Fix offset method name to reflect actual intent It was meant to return the mapping offset from the start of the file, but in reality it returned the mapping offset from the start of the page that is mapped. It is now renamed to reflect this. --- include/mio/mmap.hpp | 14 +++++++------- include/mio/shared_mmap.hpp | 6 ------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/include/mio/mmap.hpp b/include/mio/mmap.hpp index ddb95b2..def559a 100644 --- a/include/mio/mmap.hpp +++ b/include/mio/mmap.hpp @@ -180,11 +180,11 @@ public: size_type length() const noexcept { return length_; } size_type mapped_length() const noexcept { return mapped_length_; } - /** - * Returns the offset, relative to the file's start, at which the mapping was - * requested to be created. - */ - size_type offset() const noexcept { return mapped_length_ - length_; } + /** Returns the offset relative to the start of the mapping. */ + size_type mapping_offset() const noexcept + { + return mapped_length_ - length_; + } /** * Returns a pointer to the first requested byte, or `nullptr` if no memory mapping @@ -362,12 +362,12 @@ private: typename = typename std::enable_if::type > pointer get_mapping_start() noexcept { - return !data() ? nullptr : data() - offset(); + return !data() ? nullptr : data() - mapping_offset(); } const_pointer get_mapping_start() const noexcept { - return !data() ? nullptr : data() - offset(); + return !data() ? nullptr : data() - mapping_offset(); } /** diff --git a/include/mio/shared_mmap.hpp b/include/mio/shared_mmap.hpp index b914c18..f125a59 100644 --- a/include/mio/shared_mmap.hpp +++ b/include/mio/shared_mmap.hpp @@ -162,12 +162,6 @@ public: return pimpl_ ? pimpl_->mapped_length() : 0; } - /** - * Returns the offset, relative to the file's start, at which the mapping was - * requested to be created. - */ - size_type offset() const noexcept { return pimpl_ ? pimpl_->offset() : 0; } - /** * Returns a pointer to the first requested byte, or `nullptr` if no memory mapping * exists. From f75520c7196aade742ba82bb82ac0b51513a86e5 Mon Sep 17 00:00:00 2001 From: mandreyel Date: Mon, 27 May 2019 18:42:32 +0200 Subject: [PATCH 3/3] Update single include header --- single_include/mio/mio.hpp | 43 ++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/single_include/mio/mio.hpp b/single_include/mio/mio.hpp index 1d43307..b4b8cd5 100644 --- a/single_include/mio/mio.hpp +++ b/single_include/mio/mio.hpp @@ -155,15 +155,17 @@ private: // Points to the first requested byte, and not to the actual start of the mapping. pointer data_ = nullptr; - // Length, in bytes, requested by user, which may not be the length of the full - // mapping, and the entire length of the full mapping. + // Length--in bytes--requested by user (which may not be the length of the + // full mapping) and the length of the full mapping. size_type length_ = 0; size_type mapped_length_ = 0; - // Letting user map a file using both an existing file handle and a path introcudes - // On POSIX, we only need a file handle to create a mapping, while on Windows - // systems the file handle is necessary to retrieve a file mapping handle, but any - // subsequent operations on the mapped region must be done through the latter. + // Letting user map a file using both an existing file handle and a path + // introcudes some complexity (see `is_handle_internal_`). + // On POSIX, we only need a file handle to create a mapping, while on + // Windows systems the file handle is necessary to retrieve a file mapping + // handle, but any subsequent operations on the mapped region must be done + // through the latter. handle_type file_handle_ = INVALID_HANDLE_VALUE; #ifdef _WIN32 handle_type file_mapping_handle_ = INVALID_HANDLE_VALUE; @@ -173,7 +175,7 @@ private: // introcudes some complexity in that we must not close the file handle if // user provided it, but we must close it if we obtained it using the // provided path. For this reason, this flag is used to determine when to - // close file_handle_. + // close `file_handle_`. bool is_handle_internal_; public: @@ -257,11 +259,11 @@ public: size_type length() const noexcept { return length_; } size_type mapped_length() const noexcept { return mapped_length_; } - /** - * Returns the offset, relative to the file's start, at which the mapping was - * requested to be created. - */ - size_type offset() const noexcept { return mapped_length_ - length_; } + /** Returns the offset relative to the start of the mapping. */ + size_type mapping_offset() const noexcept + { + return mapped_length_ - length_; + } /** * Returns a pointer to the first requested byte, or `nullptr` if no memory mapping @@ -439,12 +441,12 @@ private: typename = typename std::enable_if::type > pointer get_mapping_start() noexcept { - return !data() ? nullptr : data() - offset(); + return !data() ? nullptr : data() - mapping_offset(); } const_pointer get_mapping_start() const noexcept { - return !data() ? nullptr : data() - offset(); + return !data() ? nullptr : data() - mapping_offset(); } /** @@ -1140,9 +1142,10 @@ void basic_mmap::unmap() if(data_) { ::munmap(const_cast(get_mapping_start()), mapped_length_); } #endif - // If file_handle_ was obtained by our opening it (when map is called with a path, - // rather than an existing file handle), we need to close it, otherwise it must not - // be closed as it may still be used outside this instance. + // If `file_handle_` was obtained by our opening it (when map is called with + // a path, rather than an existing file handle), we need to close it, + // otherwise it must not be closed as it may still be used outside this + // instance. if(is_handle_internal_) { #ifdef _WIN32 @@ -1501,12 +1504,6 @@ public: return pimpl_ ? pimpl_->mapped_length() : 0; } - /** - * Returns the offset, relative to the file's start, at which the mapping was - * requested to be created. - */ - size_type offset() const noexcept { return pimpl_ ? pimpl_->offset() : 0; } - /** * Returns a pointer to the first requested byte, or `nullptr` if no memory mapping * exists.