Merge pull request #14 from ModOrganizer2/Develop

Stage for release 2.2.1

Author: Silarn

.gitmodules

@@ -9,4 +9,4 @@
 	url = https://github.com/fmtlib/fmt.git
 [submodule "spdlog"]
 	path = spdlog
-	url = https://github.com/TanninOne/spdlog.git
+	url = https://github.com/gabime/spdlog.git

appveyor.yml

@@ -1,12 +1,12 @@
 version: 1.0.{build}
-image: Visual Studio 2017
+image: Visual Studio 2019 Preview
 configuration: Release
 platform:
 - x64
 - x86
 environment:
   GTEST_PATH: C:\Libraries\googletest
-  BOOST_PATH: C:\Libraries\boost_1_69_0
+  BOOST_PATH: C:\Libraries\boost_1_70_0
   WEBHOOK_URL:
     secure: gOKbXaZM9ImtMD5XrYITvdyZUW/az082G9OIN1EC1VZ2CuYaUUM6WY2eiNxaFeOL7/9Jyu/m+Vm1fH54CEyigcUUaxA7d8F5IMWlOgE/7YYdaAFSMUTFD7EK+++3FBYfmU1F/nZ61wsiWE6hB9Au5FpGBSCeQ0Tf8U8m0ybPmD0=
 before_build:

include/usvfs_version.h

@@ -2,7 +2,7 @@
 
 #define USVFS_VERSION_MAJOR 0
 #define USVFS_VERSION_MINOR 4
-#define USVFS_VERSION_BUILD 3
+#define USVFS_VERSION_BUILD 4
 #define USVFS_VERSION_REVISION 0
 
 #define USVFS_BUILD_STRING ""

spdlog

@@ -449,7 +449,14 @@ class DirectoryTree
     if (auto par = parent()) {
       spdlog::get("usvfs")->info("remove from tree {}", m_Name.c_str());
       auto self = par->m_Nodes.find(m_Name.c_str());
-      par->erase(self);
+      if (self != par->m_Nodes.end()) {
+        par->erase(self);
+      }
+      else {
+        //trying to remove a node that des not exist, most likely because it was already removed in a lower level call. 
+        //this is known to happen when MoveFile has the MOVEFILE_COPY_ALLOWED flag and moving a mapped file.
+        spdlog::get("usvfs")->warn("Failed to remove inexisting node from tree: {}", m_Name.c_str());
+      }
     }
   }
 

src/shared/directory_tree.h

@@ -176,15 +176,13 @@ void spdlog::sinks::shm_sink::output(level::level_enum lev,
   switch (lev) {
     case level::trace:
     case level::debug:
-    case level::notice: {
+    case level::info: {
       // m_LogQueue.send(message.c_str(), count, 0);
       sent = m_LogQueue.try_send(message.c_str(),
                                  static_cast<unsigned int>(count), 0);
     } break;
-    case level::alert:
-    case level::critical:
-    case level::emerg:
-    case level::err: {
+    case level::err:
+    case level::critical: {
       m_LogQueue.send(message.c_str(), static_cast<unsigned int>(count), 0);
     } break;
     default: {

src/shared/shmlogger.cpp

@@ -27,6 +27,7 @@ along with usvfs. If not, see <http://www.gnu.org/licenses/>.
 #include <boost/format.hpp>
 #include <atomic>
 #include <spdlog.h>
+#include <fmt/ostr.h>
 #include <cstdint>
 
 typedef boost::interprocess::message_queue_t<usvfs::shared::VoidPointerT> message_queue_interop;

src/shared/shmlogger.h

@@ -139,6 +139,7 @@ HMODULE WINAPI usvfs::hook_LoadLibraryExW(LPCWSTR lpFileName, HANDLE hFile,
   HMODULE res = nullptr;
 
   HOOK_START_GROUP(MutExHookGroup::LOAD_LIBRARY)
+  // Why is the usual if (!callContext.active()... check missing?
 
   RerouteW reroute = RerouteW::create(READ_CONTEXT(), callContext, lpFileName);
   PRE_REALCALL
@@ -488,6 +489,7 @@ DWORD WINAPI usvfs::hook_SetFileAttributesW(
   DWORD res = 0UL;
 
   HOOK_START_GROUP(MutExHookGroup::FILE_ATTRIBUTES)
+  // Why is the usual if (!callContext.active()... check missing?
 
   RerouteW reroute = RerouteW::create(READ_CONTEXT(), callContext, lpFileName);
   PRE_REALCALL
@@ -508,6 +510,7 @@ BOOL WINAPI usvfs::hook_DeleteFileW(LPCWSTR lpFileName)
   BOOL res = FALSE;
 
   HOOK_START_GROUP(MutExHookGroup::DELETE_FILE)
+  // Why is the usual if (!callContext.active()... check missing?
 
   RerouteW reroute = RerouteW::create(READ_CONTEXT(), callContext, lpFileName);
 
@@ -800,6 +803,9 @@ BOOL WINAPI usvfs::hook_MoveFileWithProgressA(LPCSTR lpExistingFileName, LPCSTR
 
 BOOL WINAPI usvfs::hook_MoveFileWithProgressW(LPCWSTR lpExistingFileName, LPCWSTR lpNewFileName, LPPROGRESS_ROUTINE lpProgressRoutine, LPVOID lpData, DWORD dwFlags)
 {
+
+  //TODO: Remove all redundant hooks to moveFile alternatives.
+  //it would appear that all other moveFile functions end up calling this one with no additional code.
   BOOL res = FALSE;
 
   HOOK_START_GROUP(MutExHookGroup::SHELL_FILEOP)
@@ -861,7 +867,9 @@ BOOL WINAPI usvfs::hook_MoveFileWithProgressW(LPCWSTR lpExistingFileName, LPCWST
   writeReroute.updateResult(callContext, res);
 
   if (res) {
-    readReroute.removeMapping(READ_CONTEXT(), isDirectory); // Updating the rerouteCreate to check deleted file entries should make this okay
+    //TODO: this call causes the node to be removed twice in case of MOVEFILE_COPY_ALLOWED as the deleteFile hook lower level already takes care of it,
+    //but deleteFile can't be disabled since we are relying on it in case of MOVEFILE_REPLACE_EXISTING for the destination file. 
+    readReroute.removeMapping(READ_CONTEXT(), isDirectory); // Updating the rerouteCreate to check deleted file entries should make this okay (not related to comments above)
 
     if (writeReroute.newReroute()) {
       if (isDirectory)
@@ -1094,6 +1102,7 @@ DLLEXPORT BOOL WINAPI usvfs::hook_RemoveDirectoryW(
   BOOL res = FALSE;
 
   HOOK_START_GROUP(MutExHookGroup::DELETE_FILE)
+  // Why is the usual if (!callContext.active()... check missing?
 
   RerouteW reroute = RerouteW::create(READ_CONTEXT(), callContext, lpPathName);
 

src/usvfs_dll/hooks/kernel32.cpp

@@ -1075,6 +1075,7 @@ NTSTATUS ntdll_mess_NtOpenFile(PHANDLE FileHandle,
   NTSTATUS res = STATUS_NO_SUCH_FILE;
 
   HOOK_START_GROUP(MutExHookGroup::OPEN_FILE)
+  // Why is the usual if (!callContext.active()... check missing?
 
   bool storePath = false;
   if (((OpenOptions & FILE_DIRECTORY_FILE) != 0UL)
@@ -1299,6 +1300,10 @@ NTSTATUS WINAPI usvfs::hook_NtClose(HANDLE Handle)
 //      std::lock_guard<std::recursive_mutex> lock(activeSearches.queryMutex);
       auto iter = activeSearches.info.find(Handle);
       if (iter != activeSearches.info.end()) {
+        if (iter->second.currentSearchHandle != INVALID_HANDLE_VALUE) {
+          ::CloseHandle(iter->second.currentSearchHandle);
+        }
+
         activeSearches.info.erase(iter);
         log = true;
       }
@@ -1340,6 +1345,7 @@ NTSTATUS WINAPI usvfs::hook_NtQueryAttributesFile(
   NTSTATUS res = STATUS_SUCCESS;
 
   HOOK_START_GROUP(MutExHookGroup::FILE_ATTRIBUTES)
+  // Why is the usual if (!callContext.active()... check missing?
 
   UnicodeString inPath = CreateUnicodeString(ObjectAttributes);
 

src/usvfs_dll/hooks/ntdll.cpp

@@ -116,7 +116,7 @@ int main(int argc, char **argv) {
       exceptionDialog(__LINE__, 2, e.what(), e2.what());
       // no way to log this
     } catch (const std::exception &) {
-      logger->critical() << e.what();
+      logger->critical(e.what());
     }
     return 1;
   }
@@ -216,7 +216,7 @@ int main(int argc, char **argv) {
       // no way to log this
       exceptionDialog(__LINE__, 2, e.what(), e2.what());
     } catch (const std::exception &) {
-      logger->critical() << e.what();
+      logger->critical(e.what());
     }
   }
 }