kadircet added inline comments.
================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:48 + return llvm::None; + } else { + DirectoryWatcher::Event Front = Events.front(); ---------------- nit: get rid of the else ================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:156 + + for (char *p = Buf; p < Buf + NumRead;) { + if (p + sizeof(struct inotify_event) > Buf + NumRead) { ---------------- naming, it should be capital `P` ================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:211 + // ... inotify-originated events processing ever after. + while (true) { + bool GotEvent = false; ---------------- `while (!Stop)` ? ================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:211 + // ... inotify-originated events processing ever after. + while (true) { + bool GotEvent = false; ---------------- kadircet wrote: > `while (!Stop)` ? why the loop always tries to empty the queue? what would be broken if we simply stopped if `Stop` was set? Also if that is important current implementation doesn't seem to be achiving that, since some events can be pushed to the queue after while loop exits. ================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:223 + break; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } ---------------- maybe add a condition variable for queue state and waint on it? ================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:244 + ReceivingThread([this]() { EventReceivingLoop(); }) { + while (WaitForInitialSync && !Stop && !FinishedInitScan) { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); ---------------- why not use two condition variables for stop and finishedinitscan? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits