Added a proper unit test for the new section_handle, and in so doing found and fixed many bugs.

11 files changed, 80 insertions, 26 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 78c5552c..947d21a7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -49,7 +49,7 @@ all_compile_features(PUBLIC
   cxx_noexcept
   cxx_constexpr
   cxx_thread_local
-  #cxx_init_captures                      ## Not supported yet by cmake 3.6
+  cxx_lambda_init_captures
   cxx_attributes
   cxx_generic_lambdas
 )
@@ -59,7 +59,7 @@ if(NOT MSVC OR CMAKE_VERSION VERSION_GREATER 3.59)
   )
 endif()
 # Set the library dependencies this library has
-all_link_libraries(PUBLIC boost-lite_hl outcome_hl)
+all_link_libraries(PUBLIC boost-lite_hl outcome_hl kerneltest_hl)
 # Set any macros this library requires
 if(WIN32)
   all_compile_definitions(PRIVATE _WIN32_WINNT=0x600)  ## Target WinVista
diff --git a/include/boost/afio/boost-lite b/include/boost/afio/boost-lite
-Subproject 1fb23154efc58f747ff4778801ee4cdbc7c74ca
+Subproject 9c5efbb8ae4a2c6cf9b0a69b6247d24807b343a
diff --git a/include/boost/afio/outcome b/include/boost/afio/outcome
-Subproject 058f256343adf44bbfd924f14c7c8bf7a779f8c
+Subproject 876acd5d1d4daf1342ab93c10f56f08de47c653
diff --git a/include/boost/afio/v2.0/detail/impl/windows/handle.ipp b/include/boost/afio/v2.0/detail/impl/windows/handle.ipp
index 2603604b..3c80a3fa 100644
--- a/include/boost/afio/v2.0/detail/impl/windows/handle.ipp
+++ b/include/boost/afio/v2.0/detail/impl/windows/handle.ipp
@@ -53,6 +53,7 @@ handle::~handle()
     if(ret.has_error())
     {
       BOOST_AFIO_LOG_FATAL(_v.h, "handle::~handle() close failed");
+      abort();
     }
   }
 }
diff --git a/include/boost/afio/v2.0/detail/impl/windows/import.hpp b/include/boost/afio/v2.0/detail/impl/windows/import.hpp
index 9f8fa7a0..245eead3 100644
--- a/include/boost/afio/v2.0/detail/impl/windows/import.hpp
+++ b/include/boost/afio/v2.0/detail/impl/windows/import.hpp
@@ -579,9 +579,9 @@ namespace windows_nt_kernel
     if(!AdjustTokenPrivileges)
       if(!(AdjustTokenPrivileges = (AdjustTokenPrivileges_t) GetProcAddress(advapi32, "AdjustTokenPrivileges")))
         abort();
+    // Only provided on Windows 8 and above
     if(!PrefetchVirtualMemory_)
-      if(!(PrefetchVirtualMemory_ = (PrefetchVirtualMemory_t) GetProcAddress(kernel32, "PrefetchVirtualMemory ")))
-        abort();
+      PrefetchVirtualMemory_ = (PrefetchVirtualMemory_t) GetProcAddress(kernel32, "PrefetchVirtualMemory");
 #ifdef BOOST_AFIO_OP_STACKBACKTRACEDEPTH
     if(dbghelp)
     {
diff --git a/include/boost/afio/v2.0/detail/impl/windows/map_handle.ipp b/include/boost/afio/v2.0/detail/impl/windows/map_handle.ipp
index 05901baa..d4588bd8 100644
--- a/include/boost/afio/v2.0/detail/impl/windows/map_handle.ipp
+++ b/include/boost/afio/v2.0/detail/impl/windows/map_handle.ipp
@@ -46,6 +46,9 @@ result<section_handle> section_handle::section(file_handle &backing, extent_type
     maximum_size = length;
   }
   maximum_size = utils::round_up_to_page_size(maximum_size);
+  // On Windows, no protection means reserve don't commit
+  if(!_flag)
+    _flag = flag::nocommit;
   result<section_handle> ret(section_handle(native_handle_type(), backing.is_valid() ? &backing : nullptr, maximum_size, _flag));
   native_handle_type &nativeh = ret.get()._v;
   ACCESS_MASK access = SECTION_QUERY;
@@ -55,24 +58,34 @@ result<section_handle> section_handle::section(file_handle &backing, extent_type
   if(_flag & flag::read)
   {
     access |= SECTION_MAP_READ;
-    prot = PAGE_READONLY;
     nativeh.behaviour |= native_handle_type::disposition::readable;
   }
   if(_flag & flag::write)
   {
     access |= SECTION_MAP_WRITE;
-    prot = PAGE_READWRITE;
     nativeh.behaviour |= native_handle_type::disposition::writable;
   }
   if(_flag & flag::execute)
   {
     access |= SECTION_MAP_EXECUTE;
-    prot = PAGE_EXECUTE;
   }
-  if(_flag & flag::cow)
+  if(!!(_flag & flag::cow) && !!(_flag & flag::execute))
+    prot = PAGE_EXECUTE_WRITECOPY;
+  else if(_flag & flag::execute)
+    prot = PAGE_EXECUTE;
+  else if(_flag & flag::cow)
     prot = PAGE_WRITECOPY;
+  else if(_flag & flag::write)
+    prot = PAGE_READWRITE;
+  else if(_flag & flag::read)
+    prot = PAGE_READONLY;
+  else
+    prot = PAGE_NOACCESS;
   if(_flag & flag::nocommit)
+  {
     attribs = SEC_RESERVE;
+    prot = PAGE_READONLY;  // Windows doesn't permit PAGE_NOACCESS for reservations
+  }
   else
     attribs = SEC_COMMIT;
   if(_flag & flag::executable)
@@ -86,9 +99,11 @@ result<section_handle> section_handle::section(file_handle &backing, extent_type
   // InitializeObjectAttributes(&ObjectAttributes, &NULL, 0, NULL, NULL);
   LARGE_INTEGER _maximum_size;
   _maximum_size.QuadPart = maximum_size;
-  NTSTATUS ntstat = NtCreateSection(&nativeh.h, access, NULL, &_maximum_size, prot, attribs, backing.is_valid() ? backing.native_handle().h : NULL);
+  HANDLE h;
+  NTSTATUS ntstat = NtCreateSection(&h, access, NULL, &_maximum_size, prot, attribs, backing.is_valid() ? backing.native_handle().h : NULL);
   if(STATUS_SUCCESS != ntstat)
     return make_errored_result_nt<section_handle>(ntstat);
+  nativeh.h = h;
   return ret;
 }
 
@@ -116,6 +131,7 @@ map_handle::~map_handle()
     if(ret.has_error())
     {
       BOOST_AFIO_LOG_FATAL(_v.h, "map_handle::~map_handle() close failed");
+      abort();
     }
   }
 }
@@ -214,7 +230,7 @@ result<map_handle::buffer_type> map_handle::commit(buffer_type region, section_h
   DWORD prot = 0;
   if(_flag == section_handle::flag::none)
   {
-    BOOST_OUTCOME_FILTER_ERROR(_region, dontneed(region));
+    BOOST_OUTCOME_FILTER_ERROR(_region, do_not_store(region));
     if(!VirtualProtect(_region.first, _region.second, PAGE_NOACCESS, NULL))
       return make_errored_result<buffer_type>(GetLastError());
     return _region;
@@ -264,7 +280,7 @@ result<span<map_handle::buffer_type>> map_handle::prefetch(span<buffer_type> reg
   return regions;
 }
 
-result<map_handle::buffer_type> map_handle::dontneed(buffer_type region) noexcept
+result<map_handle::buffer_type> map_handle::do_not_store(buffer_type region) noexcept
 {
   region = utils::round_to_page_size(region);
   if(!VirtualAlloc(region.first, region.second, MEM_RESET, 0))
diff --git a/include/boost/afio/v2.0/handle.hpp b/include/boost/afio/v2.0/handle.hpp
index 65bb9683..ed522d7f 100644
--- a/include/boost/afio/v2.0/handle.hpp
+++ b/include/boost/afio/v2.0/handle.hpp
@@ -362,8 +362,11 @@ public:
 public:
   //! Default constructor
   constexpr io_handle() = default;
-  //! Same constructors as handle
-  using handle::handle;
+  //! Construct a handle from a supplied native handle
+  BOOST_CXX14_CONSTEXPR io_handle(native_handle_type h, caching caching = caching::none, flag flags = flag::none)
+      : handle(h, caching, flags)
+  {
+  }
   //! Explicit conversion from handle permitted
   explicit io_handle(handle &&o) noexcept : handle(std::move(o)) {}
   using handle::really_copy;
diff --git a/include/boost/afio/v2.0/map_handle.hpp b/include/boost/afio/v2.0/map_handle.hpp
index 042f0219..19d24293 100644
--- a/include/boost/afio/v2.0/map_handle.hpp
+++ b/include/boost/afio/v2.0/map_handle.hpp
@@ -86,7 +86,7 @@ public:
   }
   //! Construct a section handle using the given native handle type for the section and the given i/o handle for the backing storage
   explicit section_handle(native_handle_type sectionh, io_handle *backing, extent_type maximum_size, flag __flag)
-      : handle(sectionh)
+      : handle(sectionh, handle::caching::all)
       , _backing(backing)
       , _length(maximum_size)
       , _flag(__flag)
@@ -175,6 +175,9 @@ inline std::ostream &operator<<(std::ostream &s, const section_handle::flag &v)
 \note The native handle returned by this map handle is always that of the backing storage, but closing this handle
 does not close that of the backing storage, nor does releasing this handle release that of the backing storage.
 Locking byte ranges of this handle is therefore equal to locking byte ranges in the original backing storage.
+
+\todo MADV_NOSYNC on FreeBSD needs to applied when the file is temporary
+\todo MADV_FREE on FreeBSD seems to do what MADV_DONTNEED does on Linux, investigate.
 */
 class BOOST_AFIO_DECL map_handle : public io_handle
 {
@@ -280,7 +283,7 @@ public:
 
   //! Ask the system to unset the dirty flag for the memory represented by the buffer. This will prevent any changes not yet sent to the backing storage from being sent in the future, also if the system kicks out this page and reloads it you may see some edition of the underlying storage instead of what was here. addr
   //! and length should be page aligned (see utils::page_sizes()), if not the returned buffer is the region actually undirtied.
-  static result<buffer_type> dontneed(buffer_type region) noexcept;
+  static result<buffer_type> do_not_store(buffer_type region) noexcept;
 
   /*! \brief Read data from the mapped view.
 
diff --git a/test/kerneltest b/test/kerneltest
-Subproject a96c2e797c4cf2d4e292c46d2c2d3d37df9cb6e
+Subproject d07a2d2198edf35d2f16b9d9894166e19004ddd
diff --git a/test/tests/section_handle_create_close/kernel_section_handle.cpp.hpp b/test/tests/section_handle_create_close/kernel_section_handle.cpp.hpp
index 3bf8c91b..50b4d70d 100644
--- a/test/tests/section_handle_create_close/kernel_section_handle.cpp.hpp
+++ b/test/tests/section_handle_create_close/kernel_section_handle.cpp.hpp
@@ -10,8 +10,6 @@ namespace section_handle_create_close
   BOOST_AFIO_TEST_KERNEL_DECL boost::outcome::result<boost::afio::section_handle> test_kernel_section_handle(boost::afio::file_handle &backing, boost::afio::section_handle::extent_type maximum_size, boost::afio::section_handle::flag m)
   {
     auto h = boost::afio::section_handle::section(backing, maximum_size, m);
-    if(h)
-      h.get().close();
     return h;
   }
 }
diff --git a/test/tests/section_handle_create_close/runner.cpp b/test/tests/section_handle_create_close/runner.cpp
index dd9bfb33..94f047c6 100644
--- a/test/tests/section_handle_create_close/runner.cpp
+++ b/test/tests/section_handle_create_close/runner.cpp
@@ -12,25 +12,58 @@ template <class U> inline void section_handle_create_close_(U &&f)
   using namespace BOOST_AFIO_V2_NAMESPACE;
 
   // Create a temporary file and put some text into it
-  auto temph = file_handle::file("tempfile", file_handle::mode::write, file_handle::creation::if_needed).get();
-  temph.write(0, "niall is not here", 17).get();
+  file_handle temph;
   auto boundf = [&](auto... pars) { return f(temph, pars...); };
 
   // clang-format off
-  static const auto permuter(mt_permute_parameters<
+  static const auto permuter(st_permute_parameters<
     result<void>,                               
     parameters<                              
       typename section_handle::extent_type,
       typename section_handle::flag
     >,
-    precondition::filesystem_setup_parameters
+    precondition::filesystem_setup_parameters,
+    postcondition::custom_parameters<bool>
   >(
     {
-      // Does the mode parameter have the expected side effects?
-      {   make_ready_result<void>(), { 1, section_handle::flag::read }, { "_" } },
-    },
-    precondition::filesystem_setup()
-    ));
+      { make_ready_result<void>(),{ 1, section_handle::flag::none },{ "_" },{ false } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::read },{ "_" },{ false } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::write },{ "_" },{ false } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::cow },{ "_" },{ false } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::execute },{ "_" },{ false } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::write|section_handle::flag::nocommit },{ "_" },{ false } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::write|section_handle::flag::prefault },{ "_" },{ false } },
+      //{ make_ready_result<void>(),{ 1, section_handle::flag::write|section_handle::flag::executable },{ "_" },{ false } },
+
+      { make_ready_result<void>(),{ 1, section_handle::flag::none },{ "_" },{ true } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::read },{ "_" },{ true } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::write },{ "_" },{ true } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::cow },{ "_" },{ true } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::execute },{ "_" },{ true } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::write | section_handle::flag::nocommit },{ "_" },{ true } },
+      { make_ready_result<void>(),{ 1, section_handle::flag::write | section_handle::flag::prefault },{ "_" },{ true } },
+      //{ make_ready_result<void>(),{ 1, section_handle::flag::write|section_handle::flag::executable },{ "_" },{ true } },
+  },
+    precondition::filesystem_setup(),
+    postcondition::custom(
+      [&](auto *, auto &testreturn, size_t, int use_file_backing) {
+        if (use_file_backing)
+        {
+          temph = file_handle::file("tempfile", file_handle::mode::write, file_handle::creation::if_needed).get();
+          temph.write(0, "niall is not here", 17).get();
+        }
+        else
+          temph = file_handle();
+        return &testreturn;
+      },
+      [&](auto *testreturn) {
+        // Need to close the section and any backing file as otherwise filesystem_setup won't be able to clear up the working dir
+        if (*testreturn)
+          testreturn->get().close();
+        temph.close();
+      },
+    "check section")
+  ));
   // clang-format on
 
   auto results = permuter(boundf);