From 27286406235d3c5c51d890835f1168ddadf1c9bb Mon Sep 17 00:00:00 2001 From: Daniel Wilhelm Date: Wed, 16 Mar 2016 21:33:19 +0100 Subject: 7.9 --- zen/dir_watcher.cpp | 56 +++++++++++++++++++++++++++++------------------------ zen/dir_watcher.h | 4 ++-- zen/file_access.cpp | 25 +++++++++++++++--------- zen/i18n.h | 33 +++++++++++++++++++++++++------ 4 files changed, 76 insertions(+), 42 deletions(-) (limited to 'zen') diff --git a/zen/dir_watcher.cpp b/zen/dir_watcher.cpp index 97ecafe5..e7748519 100644 --- a/zen/dir_watcher.cpp +++ b/zen/dir_watcher.cpp @@ -39,7 +39,7 @@ class SharedData { public: //context of worker thread - void addChanges(const char* buffer, DWORD bytesWritten, const Zstring& dirpath) //throw () + bool tryAddChanges(const char* buffer, DWORD bytesWritten, const Zstring& dirPathPf) //noexcept; return false on failure (already reported!) { std::lock_guard dummy(lockAccess); @@ -47,12 +47,20 @@ public: changedFiles.emplace_back(DirWatcher::ACTION_CREATE, L"Overflow."); else { - const char* bufPos = &buffer[0]; + size_t offset = 0; for (;;) { - const FILE_NOTIFY_INFORMATION& notifyInfo = reinterpret_cast(*bufPos); + const FILE_NOTIFY_INFORMATION& notifyInfo = *reinterpret_cast(buffer + offset); - const Zstring fullpath = dirpath + Zstring(notifyInfo.FileName, notifyInfo.FileNameLength / sizeof(WCHAR)); + //access-violation crash dumps suggest that the buffer can be corrupt, so let's validate: + if (offset + sizeof(FILE_NOTIFY_INFORMATION) > bytesWritten || + offset + offsetof(FILE_NOTIFY_INFORMATION, FileName) + notifyInfo.FileNameLength > bytesWritten) + { + reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), L"ReadDirectoryChangesW: corrupt data returned"); + return false; + } + + const Zstring fullpath = dirPathPf + Zstring(notifyInfo.FileName, notifyInfo.FileNameLength / sizeof(WCHAR)); [&] { @@ -85,16 +93,17 @@ public: if (notifyInfo.NextEntryOffset == 0) break; - bufPos += notifyInfo.NextEntryOffset; + offset += notifyInfo.NextEntryOffset; } } + return true; } ////context of main thread - //void addChange(const Zstring& dirpath) //throw () + //void addChange(const Zstring& dirPath) //throw () //{ // std::lock_guard dummy(lockAccess); - // changedFiles.insert(dirpath); + // changedFiles.insert(dirPath); //} @@ -105,11 +114,8 @@ public: //first check whether errors occurred in thread if (errorInfo) - { - const std::wstring msg = copyStringTo(errorInfo->msg); - const std::wstring descr = copyStringTo(errorInfo->descr); - throw FileError(msg, descr); - } + throw FileError(copyStringTo(errorInfo->msg), + copyStringTo(errorInfo->descr)); output.swap(changedFiles); changedFiles.clear(); @@ -117,10 +123,10 @@ public: //context of worker thread - void reportError(const std::wstring& msg, const std::wstring& description, DWORD errorCode) //throw() + void reportError(const std::wstring& msg, const std::wstring& description) //throw() { std::lock_guard dummy(lockAccess); - errorInfo = ErrorInfo({ copyStringTo(msg), copyStringTo(description), errorCode }); + errorInfo = ErrorInfo({ copyStringTo(msg), copyStringTo(description) }); } private: @@ -133,7 +139,6 @@ private: { BasicWString msg; BasicWString descr; - DWORD errorCode; }; Opt errorInfo; //non-empty if errors occurred in thread }; @@ -146,9 +151,9 @@ public: ReadChangesAsync(const Zstring& directory, //make sure to not leak-in thread-unsafe types! const std::shared_ptr& shared) : shared_(shared), - dirpathPf(appendSeparator(directory)) + dirPathPf(appendSeparator(directory)) { - hDir = ::CreateFile(applyLongPathPrefix(dirpathPf).c_str(), //_In_ LPCTSTR lpFileName, + hDir = ::CreateFile(applyLongPathPrefix(dirPathPf).c_str(), //_In_ LPCTSTR lpFileName, FILE_LIST_DIRECTORY, //_In_ DWORD dwDesiredAccess, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, //_In_ DWORD dwShareMode, nullptr, //_In_opt_ LPSECURITY_ATTRIBUTES lpSecurityAttributes, @@ -163,7 +168,7 @@ public: } ReadChangesAsync(ReadChangesAsync&& other) : shared_(std::move(other.shared_)), - dirpathPf(std::move(other.dirpathPf)), + dirPathPf(std::move(other.dirPathPf)), hDir(other.hDir) { other.hDir = INVALID_HANDLE_VALUE; @@ -192,7 +197,7 @@ public: if (overlapped.hEvent == nullptr) { const DWORD ec = ::GetLastError(); //copy before directly/indirectly making other system calls! - return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirpathPf)), formatSystemError(L"CreateEvent", ec), ec); + return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), formatSystemError(L"CreateEvent", ec)); } ZEN_ON_SCOPE_EXIT(::CloseHandle(overlapped.hEvent)); @@ -212,7 +217,7 @@ public: nullptr)) // __in_opt LPOVERLAPPED_COMPLETION_ROUTINE lpCompletionRoutine { const DWORD ec = ::GetLastError(); //copy before directly/indirectly making other system calls! - return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirpathPf)), formatSystemError(L"ReadDirectoryChangesW", ec), ec); + return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), formatSystemError(L"ReadDirectoryChangesW", ec)); } //async I/O is a resource that needs to be guarded since it will write to local variable "buffer"! @@ -239,7 +244,7 @@ public: { const DWORD ec = ::GetLastError(); //copy before directly/indirectly making other system calls! if (ec != ERROR_IO_INCOMPLETE) - return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirpathPf)), formatSystemError(L"GetOverlappedResult", ec), ec); + return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), formatSystemError(L"GetOverlappedResult", ec)); //execute asynchronous procedure calls (APC) queued on this thread ::SleepEx(50, // __in DWORD dwMilliseconds, @@ -249,7 +254,8 @@ public: } guardAio.dismiss(); - shared_->addChanges(&buffer[0], bytesWritten, dirpathPf); //throw () + if (!shared_->tryAddChanges(&buffer[0], bytesWritten, dirPathPf)) //noexcept + return; } } @@ -262,7 +268,7 @@ private: //shared between main and worker: std::shared_ptr shared_; //worker thread only: - Zstring dirpathPf; //thread safe! + Zstring dirPathPf; //thread safe! HANDLE hDir = INVALID_HANDLE_VALUE; }; @@ -275,7 +281,7 @@ public: InterruptibleThread& worker) : notificationHandle(registerFolderRemovalNotification(hDir, //throw FileError displayPath, - [this]{ this->onRequestRemoval (); }, //noexcept! + [this] { this->onRequestRemoval (); }, //noexcept! [this](bool successful) { this->onRemovalFinished(); })), // worker_(worker) {} @@ -549,7 +555,7 @@ void eventCallback(ConstFSEventStreamRef streamRef, struct DirWatcher::Pimpl { - FSEventStreamRef eventStream = nullptr; + FSEventStreamRef eventStream = nullptr; std::vector changedFiles; }; diff --git a/zen/dir_watcher.h b/zen/dir_watcher.h index 1d6d53cc..ddb3dbb9 100644 --- a/zen/dir_watcher.h +++ b/zen/dir_watcher.h @@ -50,10 +50,10 @@ public: struct Entry { - Entry() : action_(ACTION_CREATE) {} + Entry() {} Entry(ActionType action, const Zstring& filepath) : action_(action), filepath_(filepath) {} - ActionType action_; + ActionType action_ = ACTION_CREATE; Zstring filepath_; }; diff --git a/zen/file_access.cpp b/zen/file_access.cpp index d093d7ff..009d60b2 100644 --- a/zen/file_access.cpp +++ b/zen/file_access.cpp @@ -1279,7 +1279,7 @@ void copyItemPermissions(const Zstring& sourcePath, const Zstring& targetPath, P if (procSl == ProcSymlink::DIRECT) flags |= COPYFILE_NOFOLLOW; - if (::copyfile(sourcePath.c_str(), targetPath.c_str(), 0, flags) != 0) + if (::copyfile(sourcePath.c_str(), targetPath.c_str(), nullptr, flags) != 0) THROW_LAST_FILE_ERROR(replaceCpy(replaceCpy(_("Cannot copy permissions from %x to %y."), L"%x", L"\n" + fmtPath(sourcePath)), L"%y", L"\n" + fmtPath(targetPath)), L"copyfile"); //owner is *not* copied with ::copyfile(): @@ -1498,7 +1498,7 @@ void zen::copyNewDirectory(const Zstring& sourcePath, const Zstring& targetPath, } #elif defined ZEN_MAC - /*int rv =*/ ::copyfile(sourcePath.c_str(), targetPath.c_str(), 0, COPYFILE_XATTR); + /*int rv =*/ ::copyfile(sourcePath.c_str(), targetPath.c_str(), nullptr, COPYFILE_XATTR); #endif ZEN_ON_SCOPE_FAIL(try { removeDirectorySimple(targetPath); } @@ -1578,7 +1578,7 @@ void zen::copySymlink(const Zstring& sourceLink, const Zstring& targetLink, bool if (::lstat(sourceLink.c_str(), &sourceInfo) != 0) THROW_LAST_FILE_ERROR(replaceCpy(_("Cannot read file attributes of %x."), L"%x", fmtPath(sourceLink)), L"lstat"); - if (::copyfile(sourceLink.c_str(), targetLink.c_str(), 0, COPYFILE_XATTR | COPYFILE_NOFOLLOW) != 0) + if (::copyfile(sourceLink.c_str(), targetLink.c_str(), nullptr, COPYFILE_XATTR | COPYFILE_NOFOLLOW) != 0) THROW_LAST_FILE_ERROR(replaceCpy(replaceCpy(_("Cannot copy attributes from %x to %y."), L"%x", L"\n" + fmtPath(sourceLink)), L"%y", L"\n" + fmtPath(targetLink)), L"copyfile"); setFileTimeRaw(targetLink, &sourceInfo.st_birthtimespec, sourceInfo.st_mtimespec, ProcSymlink::DIRECT); //throw FileError @@ -2284,8 +2284,15 @@ InSyncAttributes copyFileOsSpecific(const Zstring& sourceFile, //throw FileError //http://blog.plasticsfuture.org/2006/03/05/the-state-of-backup-and-cloning-tools-under-mac-os-x/ //docs: http://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man3/copyfile.3.html //source: http://www.opensource.apple.com/source/copyfile/copyfile-103.92.1/copyfile.c - if (::fcopyfile(fileIn.getHandle(), fileOut.getHandle(), 0, COPYFILE_XATTR) != 0) - THROW_LAST_FILE_ERROR(replaceCpy(replaceCpy(_("Cannot copy attributes from %x to %y."), L"%x", L"\n" + fmtPath(sourceFile)), L"%y", L"\n" + fmtPath(targetFile)), L"copyfile"); + if (::fcopyfile(fileIn.getHandle(), fileOut.getHandle(), nullptr, COPYFILE_XATTR) != 0) + { + /* + extended attributes are only optional here => ignore error + E2BIG - reference email: "FFS V7.8 on Mac with 10.11.2 ElCapitan" + EINVAL - reference email: "Error Code 22: Invalid argument (copyfile)" + */ + //THROW_LAST_FILE_ERROR(replaceCpy(replaceCpy(_("Cannot copy attributes from %x to %y."), L"%x", L"\n" + fmtPath(sourceFile)), L"%y", L"\n" + fmtPath(targetFile)), L"fcopyfile"); + } #endif fileOut.close(); //throw FileError -> optional, but good place to catch errors when closing stream! @@ -2309,10 +2316,10 @@ InSyncAttributes copyFileOsSpecific(const Zstring& sourceFile, //throw FileError #endif /* - ------------------ - |File Copy Layers| - ------------------ - copyNewFile + ------------------ + |File Copy Layers| + ------------------ + copyNewFile | copyFileOsSpecific (solve 8.3 issue on Windows) | diff --git a/zen/i18n.h b/zen/i18n.h index e58542c2..3f10ec81 100644 --- a/zen/i18n.h +++ b/zen/i18n.h @@ -28,7 +28,7 @@ namespace zen { -//implement handler to enable program wide localizations: +//implement handler to enable program-wide localizations: struct TranslationHandler { //THREAD-SAFETY: "const" member must model thread-safe access! @@ -44,7 +44,7 @@ private: TranslationHandler& operator=(const TranslationHandler&) = delete; }; -void setTranslator(std::unique_ptr&& newHandler = nullptr); //take ownership +void setTranslator(std::unique_ptr&& newHandler); //take ownership const TranslationHandler* getTranslator(); @@ -99,20 +99,41 @@ std::wstring translate(const std::wstring& singular, const std::wstring& plural, inline -std::unique_ptr& globalTranslationHandler() +const TranslationHandler*& getTranslationInstance() { - static std::unique_ptr inst; //external linkage even in header! + //avoid static destruction order fiasco: there may be accesses to "getTranslator()" during process shutdown e.g. show message in debug_minidump.cpp! + //=> use POD instead of a std::unique_ptr<>!!! + static const TranslationHandler* inst = nullptr; //external linkage even in header! return inst; } + + +struct CleanUpTranslationHandler +{ + ~CleanUpTranslationHandler() + { + const TranslationHandler*& handler = getTranslationInstance(); + assert(!handler); //clean up at a better time rather than during static destruction! potential MT issues!? + delete handler; + handler = nullptr; //getTranslator() may be called even after static objects of this translation unit are destroyed! + } +}; } inline -void setTranslator(std::unique_ptr&& newHandler) { implementation::globalTranslationHandler() = std::move(newHandler); } +void setTranslator(std::unique_ptr&& newHandler) +{ + static implementation::CleanUpTranslationHandler cuth; //external linkage even in header! + + const TranslationHandler*& handler = implementation::getTranslationInstance(); + delete handler; + handler = newHandler.release(); +} inline -const TranslationHandler* getTranslator() { return implementation::globalTranslationHandler().get(); } +const TranslationHandler* getTranslator() { return implementation::getTranslationInstance(); } } #endif //I18_N_H_3843489325044253425456 -- cgit