Skip to content
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

Add sigaction() versions that allow specify the signal by its number #2449

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ChrysoliteAzalea
Copy link

Currently, the nix::sys::signal::sigaction() function uses the Signal enum, which only covers standard signals, but not real-time signals. Using real-time signals with nix requires using the std::mem::transmute() function to assign an impossible value to the Signal enum, which is undefined behavior.

What does this PR do

In this PR, I add two new functions: nix::sys::signal::sigaction_numeric(), that uses signal numbers instead of the enum, and nix::sys::signal::sigaction_noretrieve(), which also doesn't retrieve the old signal handler by supplying the null pointer as the oldact argument (to avoid trouble if the handler has been installed by the C code).

Checklist:

  • [yes] I have read CONTRIBUTING.md
  • [yes] I have written necessary tests and rustdoc comments
  • [?] A change log has been added if this PR modifies nix's API

@SteveLauC
Copy link
Member

Hi, thanks for the work on realtime signals.

[?] A change log has been added if this PR modifies nix's API

Nix needs a CHANGELOG entry for those PRs that modify the API, please refer to CONTRIBUTING.md on how to do this.


Related to:

From my side, the thing that blocks us from supporting realtime signals is that some platforms define the signal number as constants, while other platforms use function. And signal numbers on Linux/glibc is a runtime thing since glibc uses the first 2 or 3 rt signals for internal usage. So we cannot easily add them to the existing enum Signal type.

This PR proposes APIs that use raw c_int types with sigaction(), instead of adding new interfaces as workaround, I think we should also think about how to make rt signals work with existing interfaces, e.g., as mentioned in #495, sigwait() and waitpid() will panic when encountering rt signal, I am thinking about changing enum Signal to something like this:

#[repr(transparent)]
pub struct Signal(libc::c_int);

impl Signal {
    pub const SIGINT: Signal = Signal(libc::SIGINT);

    pub fn realtime(sig_num: libc::c_int) -> Result<Self, nix::errno::Errno> {
        let min = libc::SIGRTMIN();
        let max = libc::SIGRTMAX();

        if sig_num < min || sig_num > max {
            return Err(nix::errno::Errno::EINVAL);
        }

        Ok(Signal(sig_num))
    }
}

By using a wrapper of c_int, rt signals can be accommodated in it, though we will lost the convenient pattern matching after this conversion.

cc @asomers, would like to hear your thoughts on rt signal support.

@SteveLauC
Copy link
Member

pub enum Signal {
    Standard(nix::signal::Signal),
    Realtime(libc::c_int),
}

Or maybe something like this? Though I am not sure if realtime signal is supported by all the OSes supported by Nix.

@ChrysoliteAzalea
Copy link
Author

I wonder, should I modify existing functions, or add new one to avoid breaking changes to existing applications?

@SteveLauC
Copy link
Member

avoid breaking changes to existing applications?

Breaking change is fine, the things I care about most are the usability and ergonomics of the new interfaces.

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.

None yet

2 participants