Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds inotify support to StarryOS, implementing the core inotify data structure and three syscalls (inotify_init1, inotify_add_watch, inotify_rm_watch). The implementation provides the basic infrastructure for filesystem monitoring, though the complete event notification chain from filesystem operations is not yet implemented.
Key Changes
- Introduced
InotifyInstancestruct with watch management, event queue, and poll support - Implemented three inotify syscalls with proper integration into the syscall handler
- Added event serialization with Linux-compatible binary format
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
api/src/syscall/mod.rs |
Added inotify syscall handlers and removed inotify_init1 from dummy fd list |
api/src/syscall/fs/mod.rs |
Added inotify module to fs syscall exports |
api/src/syscall/fs/inotify.rs |
Implemented three inotify syscalls with file descriptor management and validation |
api/src/file/mod.rs |
Exported InotifyInstance and added inotify module |
api/src/file/inotify.rs |
Core inotify implementation with data structures, watch management, and FileLike/Pollable trait support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
756267e to
63cc59c
Compare
api/src/file/inotify.rs
Outdated
| event_queue: Mutex<VecDeque<Vec<u8>>>, | ||
|
|
||
| // watches: wd -> (path, event mask) | ||
| watches: Mutex<BTreeMap<i32, (String, u32)>>, |
There was a problem hiding this comment.
Since the subjects of inotify are inodes (not pathnames), related data should be stored in Location::user_data instead. You should base your implementation on that API.
There was a problem hiding this comment.
Ok, I have modified my implementation.
666867e to
f87df06
Compare
api/src/file/inotify.rs
Outdated
| let location = self.resolve_path(path)?; | ||
| // Generate a new watch descriptor | ||
| let wd = { | ||
| let mut next = self.next_wd.lock(); |
There was a problem hiding this comment.
AtomicI32 would be sufficient for this usage
There was a problem hiding this comment.
Changed next_wd from Mutex<i32> to AtomicI32 per your suggestion.
f87df06 to
c9aedf9
Compare
Description
Implementation
Additional Context