Catch Ctrl-C and enable Ctrl-D #77
Conversation
|
Will this fail to build on Windows? |
|
man page says |
I honestly don't know and don't have a VM ready to test it out. Now taking no chances and simply disable catching |
Interesting, this warning is not present in the macOS man page. IIUC there wouldn't be a big behavior difference in the dummy case, I'll nonetheless switch over to use |
|
Should we just add a CI job to build on windows? Feel free to squash. |
tnull
force-pushed
the
2022-10-catch-ctrlc
branch
5 times, most recently
from
d0387d3
to
5252753
Compare
Added CI jobs for |
src/main.rs
Outdated
| let mut new_action: libc::sigaction = core::mem::zeroed(); | ||
| let mut old_action: libc::sigaction = core::mem::zeroed(); | ||
|
|
||
| new_action.sa_sigaction = libc::SIG_IGN; |
There was a problem hiding this comment.
man page seems to think you should be applying this to sa_handler, not sa_sigaction.
There was a problem hiding this comment.
Right, only thing is, sa_sigaction seems to be the only handler field available in libc?: https://docs.rs/libc/0.2.135/libc/struct.sigaction.html
Also seems to work fine.
There was a problem hiding this comment.
Hmm, probably because On some architectures a union is involved: do not assign to both sa_handler and sa_sigaction.
But also If SA_SIGINFO is specified in sa_flags, then sa_sigaction (instead of sa_handler) specifies the signal-handling function for signum. This function receives three arguments, as described below.
If we can't assign to sa_handler we should set SA_SIGINFO, assign to sa_sigaction, and use our own function rather than SIG_IGN.
There was a problem hiding this comment.
Mh, good point, I didn't consider that those two might not always be a union. I'm however a bit confused by what the current state of sigaction in libc is, since there are some parts of the code that suggest the previous way would work fine. Then again there is rust-lang/libc#2107, which seems to be blocked until the next major release (and somehow still calls the field sa_sigaction, even though also introducing a union with sa_handler).
TLDR: Just to be safe I now went the dummy handler route (and squashed again).
We ignore Ctrl-C/SIGINT via a dummy signal handler and enable the use of Ctrl-D. For convenience, we introduce "exit" and "quit" cli commands.
tnull
force-pushed
the
2022-10-catch-ctrlc
branch
from
5252753
to
e20eeb0
Compare
Fixes #76.
We catch Ctrl-C/SIGINT via a dummy signal handler and enable the use of Ctrl-D. For convenience, we introduce "exit" and "quit" cli commands.