Win: Use full Windows paths in file watcher and improve logging

diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp
index 3bc0a11df2d1b2fcd02dc434421bd38f08178efc..594817135355bd89d3d0eb81730ca9ef55c92e39 100644 (file)
--- a/src/common/filesystembase.cpp
+++ b/src/common/filesystembase.cpp
@@ -141,7 +141,7 @@ bool FileSystem::rename(const QString &originFileName,
             (wchar_t *)dest.utf16(),
             MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH);
         if (!success) {
-            error = Utility::formatWinError();
+            error = Utility::formatLastWinError();
         }
     } else
 #endif

diff --git a/src/common/utility.h b/src/common/utility.h
index 3fe99e82107ce64b1ceec01f8864524fb32c42c4..ad18c9f1cf27bded597ce5ebd2aaa0af45f89e20 100644 (file)
--- a/src/common/utility.h
+++ b/src/common/utility.h
@@ -249,7 +249,10 @@ namespace Utility {
     OCSYNC_EXPORT void FiletimeToLargeIntegerFiletime(FILETIME *filetime, LARGE_INTEGER *hundredNSecs);
     OCSYNC_EXPORT void UnixTimeToLargeIntegerFiletime(time_t t, LARGE_INTEGER *hundredNSecs);
 
-    OCSYNC_EXPORT QString formatWinError(long error = GetLastError());
+    OCSYNC_EXPORT QString formatWinError(long error);
+    inline QString formatLastWinError() {
+        return formatWinError(GetLastError());
+    };
 
 #endif
 }

diff --git a/src/gui/folderwatcher_win.cpp b/src/gui/folderwatcher_win.cpp
index 9e184c32c1d9215d888a2f1c30241de4151b1145..f152fa83b1bef5e203b4b1f83a7b90f74e89dce1 100644 (file)
--- a/src/gui/folderwatcher_win.cpp
+++ b/src/gui/folderwatcher_win.cpp
@@ -15,6 +15,8 @@
 #include <QThread>
 #include <QDir>
 
+#include "common/asserts.h"
+#include "common/utility.h"
 #include "filesystem.h"
 #include "folderwatcher.h"
 #include "folderwatcher_win.h"
@@ -31,10 +33,10 @@ void WatcherThread::watchChanges(size_t fileNotifyBufferSize,
     bool *increaseBufferSize)
 {
     *increaseBufferSize = false;
-    QString longPath = FileSystem::longWinPath(_path);
+    const QString longPath = FileSystem::longWinPath(_path);
 
     _directory = CreateFileW(
-        (wchar_t *)longPath.utf16(),
+        longPath.toStdWString().data(),
         FILE_LIST_DIRECTORY,
         FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
         nullptr,
@@ -43,8 +45,7 @@ void WatcherThread::watchChanges(size_t fileNotifyBufferSize,
         nullptr);
 
     if (_directory == INVALID_HANDLE_VALUE) {
-        DWORD errorCode = GetLastError();
-        qCWarning(lcFolderWatcher) << "Failed to create handle for" << _path << ", error:" << errorCode;
+        qCWarning(lcFolderWatcher) << "Failed to create handle for" << _path << ", error:" << Utility::formatLastWinError();
         _directory = 0;
         return;
     }
@@ -54,7 +55,7 @@ void WatcherThread::watchChanges(size_t fileNotifyBufferSize,
 
     // QVarLengthArray ensures the stack-buffer is aligned like double and qint64.
     QVarLengthArray<char, 4096 * 10> fileNotifyBuffer;
-    fileNotifyBuffer.resize(OCC::Utility::convertSizeToInt(fileNotifyBufferSize));
+    fileNotifyBuffer.resize(static_cast<int>(fileNotifyBufferSize));
 
     const size_t fileNameBufferSize = 4096;
     TCHAR fileNameBuffer[fileNameBufferSize];
@@ -64,11 +65,10 @@ void WatcherThread::watchChanges(size_t fileNotifyBufferSize,
         ResetEvent(_resultEvent);
 
         FILE_NOTIFY_INFORMATION *pFileNotifyBuffer =
-            (FILE_NOTIFY_INFORMATION *)fileNotifyBuffer.data();
+                reinterpret_cast<FILE_NOTIFY_INFORMATION *>(fileNotifyBuffer.data());
         DWORD dwBytesReturned = 0;
-        SecureZeroMemory(pFileNotifyBuffer, fileNotifyBufferSize);
-        if (!ReadDirectoryChangesW(_directory, (LPVOID)pFileNotifyBuffer,
-                OCC::Utility::convertSizeToDWORD(fileNotifyBufferSize), true,
+        if (!ReadDirectoryChangesW(_directory, pFileNotifyBuffer,
+                static_cast<DWORD>(fileNotifyBufferSize), true,
                 FILE_NOTIFY_CHANGE_FILE_NAME
                 | FILE_NOTIFY_CHANGE_DIR_NAME
                 | FILE_NOTIFY_CHANGE_LAST_WRITE
@@ -76,13 +76,13 @@ void WatcherThread::watchChanges(size_t fileNotifyBufferSize,
                 &dwBytesReturned,
                 &overlapped,
                 nullptr)) {
-            DWORD errorCode = GetLastError();
+            const DWORD errorCode = GetLastError();
             if (errorCode == ERROR_NOTIFY_ENUM_DIR) {
                 qCDebug(lcFolderWatcher) << "The buffer for changes overflowed! Triggering a generic change and resizing";
                 emit changed(_path);
                 *increaseBufferSize = true;
             } else {
-                qCWarning(lcFolderWatcher) << "ReadDirectoryChangesW error" << errorCode;
+                qCWarning(lcFolderWatcher) << "ReadDirectoryChangesW error" << Utility::formatWinError(errorCode);
             }
             break;
         }
@@ -99,47 +99,56 @@ void WatcherThread::watchChanges(size_t fileNotifyBufferSize,
             break;
         }
         if (result != 0) {
-            qCWarning(lcFolderWatcher) << "WaitForMultipleObjects failed" << result << GetLastError();
+            qCWarning(lcFolderWatcher) << "WaitForMultipleObjects failed" << result << Utility::formatLastWinError();
             break;
         }
 
         bool ok = GetOverlappedResult(_directory, &overlapped, &dwBytesReturned, false);
         if (!ok) {
-            DWORD errorCode = GetLastError();
+            const DWORD errorCode = GetLastError();
             if (errorCode == ERROR_NOTIFY_ENUM_DIR) {
                 qCDebug(lcFolderWatcher) << "The buffer for changes overflowed! Triggering a generic change and resizing";
                 emit lostChanges();
                 emit changed(_path);
                 *increaseBufferSize = true;
             } else {
-                qCWarning(lcFolderWatcher) << "GetOverlappedResult error" << errorCode;
+                qCWarning(lcFolderWatcher) << "GetOverlappedResult error" << Utility::formatWinError(errorCode);
             }
             break;
         }
 
         FILE_NOTIFY_INFORMATION *curEntry = pFileNotifyBuffer;
         forever {
-            size_t len = curEntry->FileNameLength / 2;
-            QString file = _path + "\\" + QString::fromWCharArray(curEntry->FileName, OCC::Utility::convertSizeToInt(len));
+            const int len = curEntry->FileNameLength / sizeof(wchar_t);
+            QString longfile = longPath + QString::fromWCharArray(curEntry->FileName, len);
 
             // Unless the file was removed or renamed, get its full long name
             // TODO: We could still try expanding the path in the tricky cases...
-            QString longfile = file;
             if (curEntry->Action != FILE_ACTION_REMOVED
                 && curEntry->Action != FILE_ACTION_RENAMED_OLD_NAME) {
-                size_t longNameSize = GetLongPathNameW(reinterpret_cast<LPCWSTR>(file.utf16()), fileNameBuffer, fileNameBufferSize);
+                const auto wfile = longfile.toStdWString();
+                const int longNameSize = GetLongPathNameW(wfile.data(), fileNameBuffer, fileNameBufferSize);
                 if (longNameSize > 0) {
-                    longfile = QString::fromUtf16(reinterpret_cast<const ushort *>(fileNameBuffer), OCC::Utility::convertSizeToInt(longNameSize));
+                    longfile = QString::fromWCharArray(fileNameBuffer, longNameSize);
                 } else {
-                    qCWarning(lcFolderWatcher) << "Error converting file name to full length, keeping original name.";
+                    qCWarning(lcFolderWatcher) << "Error converting file name" << longfile << "to full length, keeping original name." << Utility::formatLastWinError();
                 }
             }
+
+#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)
+            // The prefix is needed for native Windows functions before Windows 10, version 1607
+            const bool hasLongPathPrefix = longPath.startsWith(QStringLiteral("\\\\?\\"));
+            if (hasLongPathPrefix)
+            {
+                longfile.remove(0, 4);
+            }
+#endif
             longfile = QDir::cleanPath(longfile);
 
             // Skip modifications of folders: One of these is triggered for changes
             // and new files in a folder, probably because of the folder's mtime
             // changing. We don't need them.
-            bool skip = curEntry->Action == FILE_ACTION_MODIFIED
+            const bool skip = curEntry->Action == FILE_ACTION_MODIFIED
                 && QFileInfo(longfile).isDir();
 
             if (!skip) {
@@ -151,7 +160,8 @@ void WatcherThread::watchChanges(size_t fileNotifyBufferSize,
             if (curEntry->NextEntryOffset == 0) {
                 break;
             }
-            curEntry = (FILE_NOTIFY_INFORMATION *)((char *)curEntry + curEntry->NextEntryOffset);
+            // FILE_NOTIFY_INFORMATION has no fixed size and the offset is in bytes therefor we first need to cast to char
+            curEntry = reinterpret_cast<FILE_NOTIFY_INFORMATION *>(reinterpret_cast<char*>(curEntry) + curEntry->NextEntryOffset);
         }
     }
 
@@ -175,7 +185,7 @@ void WatcherThread::run()
     // If this buffer fills up before we've extracted its data we will lose
     // change information. Therefore start big.
     size_t bufferSize = 4096 * 10;
-    size_t maxBuffer = 64 * 1024;
+    const size_t maxBuffer = 64 * 1024;
 
     while (!_done) {
         bool increaseBufferSize = false;

diff --git a/src/gui/folderwatcher_win.h b/src/gui/folderwatcher_win.h
index c89bfbbae897db5f0bb3a6a42922e890ba82654d..d72665f0607d0e8bb9fd60599b564f18c0845a73 100644 (file)
--- a/src/gui/folderwatcher_win.h
+++ b/src/gui/folderwatcher_win.h
@@ -33,7 +33,7 @@ class WatcherThread : public QThread
 public:
     WatcherThread(const QString &path)
         : QThread()
-        , _path(path)
+        , _path(path + (path.endsWith(QLatin1Char('/')) ? QString() : QStringLiteral("/")))
         , _directory(0)
         , _resultEvent(0)
         , _stopEvent(0)