Skip to content

fix: implement signalfd#31

Merged
AsakuraMizu merged 1 commit intoStarry-OS:mainfrom
yoinspiration:main
Nov 19, 2025
Merged

fix: implement signalfd#31
AsakuraMizu merged 1 commit intoStarry-OS:mainfrom
yoinspiration:main

Conversation

@yoinspiration
Copy link
Copy Markdown
Contributor

  • Add Signalfd file descriptor implementation (api/src/file/signalfd.rs)
  • Implement signalfd4 system call handler (api/src/syscall/fs/signalfd.rs)
  • Register signalfd4 in syscall router (api/src/syscall/mod.rs)
  • Export signalfd module in file/mod.rs and syscall/fs/mod.rs

This fixes the panic when reading from signalfd4 file descriptor. Previously, signalfd4 returned a DummyFd which would panic on read. Now it returns a proper Signalfd that can read signals correctly.

Fixes #15

@AsakuraMizu
Copy link
Copy Markdown
Contributor

Please refer to https://docs.github.com/zh/get-started/git-basics/configuring-git-to-handle-line-endings and update your commit. I can not identify your changes in diff.

@yoinspiration yoinspiration force-pushed the main branch 2 times, most recently from 7ea31b2 to 5bd5ce2 Compare November 17, 2025 05:31
@yoinspiration
Copy link
Copy Markdown
Contributor Author

I've updated the commit according to the GitHub documentation on configuring Git to handle line endings.

What I've Done

  1. Added .gitattributes file to ensure consistent line endings (LF)
  2. Reorganized the commit to clearly show only the actual code changes

Result

The commit now displays clean, readable diffs:

  • 6 files changed, 300 insertions(+), 3 deletions(-)
  • New files: api/src/file/signalfd.rs (194 lines) and api/src/syscall/fs/signalfd.rs (65 lines)
  • Modified files show only relevant changes (e.g., api/src/file/mod.rs shows just 1 line: +pub mod signalfd;)

The diff statistics are consistent with or without whitespace ignored, confirming the line ending issue is resolved. You should now be able to clearly identify all code changes in the diff.

Copy link
Copy Markdown
Contributor

Copilot AI left a 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 implements the signalfd4 system call to fix Issue #15, which was causing a panic when reading from signalfd4 file descriptors. The implementation replaces the previous DummyFd with a fully functional Signalfd that can properly read signals.

Key changes:

  • Implements Signalfd struct with signal dequeuing and polling capabilities
  • Adds sys_signalfd4 syscall handler with proper flag handling and signal mask validation
  • Registers signalfd4 in the syscall router, removing it from the dummy fd list

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
api/src/file/signalfd.rs New file implementing the Signalfd file descriptor with read, poll, and signal handling logic
api/src/syscall/fs/signalfd.rs New file implementing sys_signalfd4 syscall handler with flag parsing and fd creation
api/src/syscall/mod.rs Registers signalfd4 syscall and removes it from dummy fd list
api/src/syscall/fs/mod.rs Exports signalfd module
api/src/file/mod.rs Exports signalfd module
.gitattributes Adds line ending normalization rules (unrelated to signalfd implementation)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yoinspiration
Copy link
Copy Markdown
Contributor Author

Thank you for the thorough review! I've addressed all the comments:

  1. SFD flag constants: Updated to use O_CLOEXEC and O_NONBLOCK from linux_raw_sys::general instead of hardcoded values, following the same pattern as pipe.rs and event.rs.

  2. Chinese comments: Translated all Chinese comments to English for consistency with the codebase.

  3. Unsafe block formatting: Removed the empty line and updated the comment to use singular "field" since only one field is accessed.

All changes have been committed and pushed. The implementation now correctly handles the signalfd4 flags and maintains consistency with the rest of the codebase.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yoinspiration
Copy link
Copy Markdown
Contributor Author

Thanks for the additional feedback! I've addressed each comment in the latest commit:

  • Updated SignalfdSiginfo to avoid packed-struct UB and added helper for the union access.
  • Signalfd now allows mask updates when fd != -1, matching the Linux spec.
  • Byte serialization uses ptr::addr_of! to avoid unaligned references.

Please let me know if anything else needs adjustment!

@yoinspiration
Copy link
Copy Markdown
Contributor Author

need to accept this pull request Starry-OS/starry-signal#5 first in starry-signal repository, or it will encounter CI errors without SignalInfo::errno() method

- Implement signalfd4 syscall to fix Issue Starry-OS#15
- Update starry-signal dependency to revision acdb5e9
- Refactor: simplify bytes conversion in Signalfd using zerocopy
- Update SignalfdSiginfo struct to use Immutable and IntoBytes derives
- Add .gitattributes file
@AsakuraMizu AsakuraMizu changed the title fix: implement signalfd4 syscall to fix Issue #15 fix: implement signalfd Nov 19, 2025
@AsakuraMizu AsakuraMizu merged commit 50c0595 into Starry-OS:main Nov 19, 2025
12 checks passed
@AsakuraMizu
Copy link
Copy Markdown
Contributor

You forgot to update Cargo.lock xd

@yoinspiration
Copy link
Copy Markdown
Contributor Author

True, my bad — thanks for updating it!

@yoinspiration
Copy link
Copy Markdown
Contributor Author

yoinspiration commented Nov 21, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

signalfd4 syscall crashes with unimplemented!() panic

3 participants