Skip to content

Commit a16a9f8

Browse files
asomersSteveLauC
authored andcommitted
Remove the PartialEq and Eq implementations from SigHandler
Because it never worked reliably anyway. See https://doc.rust-lang.org/nightly/core/ptr/fn.fn_addr_eq.html for more info. Alternatively, we could only remove `Eq` and leave `PartialEq`. We would be able to guarantee equality or inequality in most cases, but would be unable to prove that different handler functions are actually different. I think users would find that confusing. Reported by: Clippy (unpredictable_function_pointer_comparisons)
1 parent fbfb433 commit a16a9f8

File tree

3 files changed

+20
-21
lines changed

3 files changed

+20
-21
lines changed

changelog/2642.removed.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Removed `Eq` and `PartialEq` implementations from `SigHandler`, because they
2+
never worked reliably. The suggested alternative is `matches!`. For example:
3+
```
4+
let h: SigHandler = ...
5+
if matches!(h, SigHandler::SigIgn) {
6+
...
7+
}
8+
```

src/sys/signal.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ impl<'a> IntoIterator for &'a SigSet {
753753
}
754754

755755
/// A signal handler.
756-
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
756+
#[derive(Clone, Copy, Debug, Hash)]
757757
pub enum SigHandler {
758758
/// Default signal handling.
759759
SigDfl,

test/sys/test_signal.rs

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -112,32 +112,23 @@ fn test_signal() {
112112

113113
unsafe { signal(Signal::SIGINT, SigHandler::SigIgn) }.unwrap();
114114
raise(Signal::SIGINT).unwrap();
115-
assert_eq!(
116-
unsafe { signal(Signal::SIGINT, SigHandler::SigDfl) }.unwrap(),
117-
SigHandler::SigIgn
118-
);
115+
let h = unsafe { signal(Signal::SIGINT, SigHandler::SigDfl) }.unwrap();
116+
assert!(matches!(h, SigHandler::SigIgn));
119117

120118
let handler = SigHandler::Handler(test_sigaction_handler);
121-
assert_eq!(
122-
unsafe { signal(Signal::SIGINT, handler) }.unwrap(),
123-
SigHandler::SigDfl
124-
);
119+
let h = unsafe { signal(Signal::SIGINT, handler) }.unwrap();
120+
assert!(matches!(h, SigHandler::SigDfl));
125121
raise(Signal::SIGINT).unwrap();
126122
assert!(SIGNALED.load(Ordering::Relaxed));
127123

124+
let h = unsafe { signal(Signal::SIGINT, SigHandler::SigDfl) }.unwrap();
128125
#[cfg(not(solarish))]
129-
assert_eq!(
130-
unsafe { signal(Signal::SIGINT, SigHandler::SigDfl) }.unwrap(),
131-
handler
132-
);
126+
assert!(matches!(h, SigHandler::Handler(_)));
133127

134128
// System V based OSes (e.g. illumos and Solaris) always resets the
135129
// disposition to SIG_DFL prior to calling the signal handler
136130
#[cfg(solarish)]
137-
assert_eq!(
138-
unsafe { signal(Signal::SIGINT, SigHandler::SigDfl) }.unwrap(),
139-
SigHandler::SigDfl
140-
);
131+
assert!(matches!(h, SigHandler::SigDfl));
141132

142133
// Restore default signal handler
143134
unsafe { signal(Signal::SIGINT, SigHandler::SigDfl) }.unwrap();
@@ -307,21 +298,21 @@ fn test_sigaction() {
307298
action_sig.flags(),
308299
SaFlags::SA_ONSTACK | SaFlags::SA_RESTART
309300
);
310-
assert_eq!(action_sig.handler(), handler_sig);
301+
assert!(matches!(action_sig.handler(), SigHandler::Handler(_)));
311302

312303
mask = action_sig.mask();
313304
assert!(mask.contains(SIGUSR1));
314305
assert!(!mask.contains(SIGUSR2));
315306

316307
let handler_act = SigHandler::SigAction(test_sigaction_action);
317308
let action_act = SigAction::new(handler_act, flags, mask);
318-
assert_eq!(action_act.handler(), handler_act);
309+
assert!(matches!(action_act.handler(), SigHandler::SigAction(_)));
319310

320311
let action_dfl = SigAction::new(SigHandler::SigDfl, flags, mask);
321-
assert_eq!(action_dfl.handler(), SigHandler::SigDfl);
312+
assert!(matches!(action_dfl.handler(), SigHandler::SigDfl));
322313

323314
let action_ign = SigAction::new(SigHandler::SigIgn, flags, mask);
324-
assert_eq!(action_ign.handler(), SigHandler::SigIgn);
315+
assert!(matches!(action_ign.handler(), SigHandler::SigIgn));
325316
})
326317
.join()
327318
.unwrap();

0 commit comments

Comments
 (0)