-
Notifications
You must be signed in to change notification settings - Fork 53
feat(syscall): add inotify syscalls #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AtomicI32 would be sufficient for this usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed next_wd from Mutex<i32> to AtomicI32 per your suggestion.
f87df06 to
c9aedf9
Compare
Description
Implementation
Additional Context