Fix the definition of sigevent on FreeBSD and Linux #3630
Conversation
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use |
|
@JohnTitor I fixed the build failure on musl and rebased on main |
|
The Sparc failure is not due to this PR. |
|
Considering the main branch is for version 1.0, which allows breaking changes, I think we can move this forward. |
|
☔ The latest upstream changes (presumably #3962) made this pull request unmergeable. Please resolve the merge conflicts. |
It was originally defined back before rust could represent C unions. So instead of defining the union field correctly, it simply defined that union's most useful field. Define it correctly now. Remove traits that can't be safely implemented on a union: PartialEq, Eq, and Hash. Define Debug, but exclude the union field.
|
Rebased, @tgross35 |
|
I think the build failure on s390x is unrelated. |
|
That job seems to fail a lot, I wrote up #4112. Thanks for the changes, I'll take a look today. |
| __unused3: ::c_long, | ||
| __unused4: ::c_long, | ||
| __unused5: *mut ::c_void, |
There was a problem hiding this comment.
Could this maybe just be put in an __aiocb_private structure for clarity?
| __unused5: *mut ::c_void, | ||
| pub aio_sigevent: sigevent, | ||
| pub struct __c_anonymous_sigev_thread { | ||
| pub _function: *mut ::c_void, // Actually a function pointer |
There was a problem hiding this comment.
Could this be a function pointer?
pub _function: Option<extern "C" fn(sigval) -> *mut c_void>
| __policy: ::c_int, | ||
| __error_code: ::c_int, | ||
| __return_value: ::ssize_t, | ||
| pub aio_offset: off_t, |
There was a problem hiding this comment.
Since this changes depending on file64, could you add this fixme so we catch it when doing that update?
// FIXME(off64): visible fields depend on __USE_FILE_OFFSET64
pub aio_offset: off_t,
| __return_value: ::ssize_t, | ||
| pub aio_offset: off_t, | ||
| #[cfg(all(not(target_arch = "x86_64"), target_pointer_width = "32"))] | ||
| __unused1: [::c_char; 4], |
There was a problem hiding this comment.
Doesn't make much of a difference but it looks like glibc calls this __pad https://github.com/bminor/glibc/blob/d899b48a30b2dd27ab25e1cd90ce28b75f7c0755/rt/aio.h#L52
| #[cfg(target_pointer_width = "32")] | ||
| __dummy4: [::c_char; 24], | ||
| #[cfg(target_pointer_width = "64")] | ||
| __dummy4: [::c_char; 16], |
There was a problem hiding this comment.
Assuming ctest complains about using an expression here, could you add a fixme?
| #[cfg(target_pointer_width = "32")] | |
| __dummy4: [::c_char; 24], | |
| #[cfg(target_pointer_width = "64")] | |
| __dummy4: [::c_char; 16], | |
| // FIXME(ctest): length should be `32 - 2 * core::mem::size_of::<*const ()>()` | |
| #[cfg(target_pointer_width = "32")] | |
| __dummy4: [::c_char; 24], | |
| #[cfg(target_pointer_width = "64")] | |
| __dummy4: [::c_char; 16], |
| #[allow(missing_debug_implementations)] | ||
| pub union __c_anonymous_sigev_un { | ||
| #[cfg(target_pointer_width = "64")] | ||
| _pad: [::c_int; (64 - 2 * 4 - 8) / 4], |
There was a problem hiding this comment.
Could you move this to a const SIGEV_PAD_SIZE: usize = ... to match the source?
| @@ -12,6 +12,11 @@ missing! { | |||
| } | |||
|
|
|||
| s! { | |||
| pub struct __c_anonymous_sigev_thread { | |||
| pub _function: *mut ::c_void, // Actually a function pointer | |||
There was a problem hiding this comment.
| pub _function: *mut ::c_void, // Actually a function pointer | |
| pub _function: Option<extern "C" fn(::sigval_t) -> *mut ::c_void>` |
|
I went through line by line, everything looks correct here but I left a handful of consistency comments. |
It was originally defined back before rust could represent C unions. So instead of defining the union field correctly, it simply defined that union's most useful field. Define it correctly now.
Remove traits that can't be safely implemented on a union: PartialEq, Eq, and Hash. Define Debug, but exclude the union field. Leave Clone in place.
This PR is a rebase of #2813 that eschews all of the backwards-compatibility parts.