Implement safe bindings around pthread_sigqueue. #1798
Conversation
pirocks
force-pushed
the
master
branch
5 times, most recently
from
525eca9 to
070abb9
Compare
|
No idea how review requests here, but other people seem to request reviews from @rtzoeller. This should be good to go for review. |
test/sys/test_timerfd.rs
Outdated
| @@ -65,5 +65,5 @@ pub fn test_timerfd_unset() { | |||
|
|
|||
| timer.unset().unwrap(); | |||
|
|
|||
| assert!(timer.get().unwrap() == None); | |||
There was a problem hiding this comment.
This part looks like a merge error. You should revert it.
There was a problem hiding this comment.
Removing the commit that does this causes this CI failure: https://github.com/nix-rust/nix/pull/1798/checks?check_run_id=7898677574
| Ptr(*mut libc::c_void), | ||
| } | ||
|
|
||
| // Because of macro/trait machinery in libc, libc doesn't provide this union. |
There was a problem hiding this comment.
Why not? It's possible to define unions in libc these days. Unless there is a very compelling reason not to, you should submit this as a PR for libc.
There was a problem hiding this comment.
The problem is there already is a sigval in libc and is defined as a struct(which is used in many places since signal handling has several syscalls associated with it). That struct has PartialEq/Hash/Debug etc implemented, and has done so for a long time. There's no practical way to implement PartialEq/Hash/Debug for a union b/c that would lead to hashing/equality checking/printing uninitialized memory. So changing the current struct to a union is a breaking change. (see: rust-lang/libc#2816)
I can add a second definition in libc, but it would have to have a different name. If that's desired I guess reply to this comment. I'll also update that comment with the info in this one.
There was a problem hiding this comment.
Have you seen rust-lang/libc#2813 ? It deals with another union that was originally defined as a struct in libc. The solution we came up with there was to define a sigevent_0_2_126 struct identical to the old sigevent, and then define sigevent with union fields as it ought to be. Then implement Deref and DerefMut for sigevent to sigevent_0_2_126 . That way consumers of the old version can still access the fields that should've been in a union. Would something similar work for sigval?
src/sys/pthread.rs
Outdated
| /// `pthread_sigqueue` is a GNU extension and is not available on other libcs | ||
| /// | ||
| /// [`pthread_sigqueue(3)`]: https://man7.org/linux/man-pages/man3/pthread_sigqueue.3.html | ||
| #[cfg(all(any(target_os = "linux", target_os = "android"), target_env = "gnu"))] |
There was a problem hiding this comment.
You can simplify this to
| #[cfg(all(any(target_os = "linux", target_os = "android"), target_env = "gnu"))] | |
| #[cfg(target_env = "gnu")] |
And likewise in the test
Co-authored-by: Alan Somers <[email protected]>
This reverts commit a79d7ed.
|
I'll rebase those review comments once every thread is resolved. |
I had a usecase for pthread_sigqueue so I added it to nix.
Implementation is pretty much the same as pthread_kill.
Requires a libc version bump.