Skip to content

Conversation

rmehri01
Copy link
Contributor

@rmehri01 rmehri01 commented Aug 11, 2025

Description

  • Simplify the check for whether we should use repr(packed) on epoll_event to be compliant with the below sources
    • It looks like musl, android, and uapi all only use packed on x86_64, but glibc only uses packed on x86 so we can do any(target_arch = "x86_64", all(target_arch = "x86", target_env = "gnu"))

Closes #4628

Sources

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot rustbot added O-linux-like O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Aug 11, 2025
@rmehri01 rmehri01 force-pushed the epoll_event_packed_musl branch from ed6cdf9 to ce34afb Compare August 11, 2025 17:07
@rmehri01
Copy link
Contributor Author

rmehri01 commented Aug 11, 2025

Ah this seems to fail on i686 which is 32-bit so maybe this check is already correct, though maybe making the check #[cfg_attr(target_arch = "x86_64", repr(packed))] instead would be more reflective of https://github.com/bminor/glibc/blob/8543577b04ded6d979ffcc5a818930e4d74d0645/sysdeps/unix/sysv/linux/sys/epoll.h#L85-L89

@tgross35
Copy link
Contributor

Looking around:

So I think it would be most accurate to update this to cfg(any(target_arch = "x86_64", target_env = "gnu"))

@rmehri01
Copy link
Contributor Author

@rmehri01 rmehri01 force-pushed the epoll_event_packed_musl branch from ce34afb to fa3fdaa Compare August 11, 2025 20:43
@rmehri01 rmehri01 changed the title musl: make epoll_event packed on x86 simplify epoll_event packed check Aug 11, 2025
@tgross35
Copy link
Contributor

tgross35 commented Aug 11, 2025

Oh yup you're right, I should have checked the __EPOLL_PACKED def (I was just looking at that yesterday too). The change here should be any(target_arch = "x86_64", all(target_arch = "x86", target_env = "gnu")) then (not enabled on all gnu targets)

@rmehri01 rmehri01 force-pushed the epoll_event_packed_musl branch 2 times, most recently from 93bcdc6 to c33fc9f Compare August 11, 2025 20:57
@rmehri01 rmehri01 force-pushed the epoll_event_packed_musl branch from c33fc9f to 7d54be1 Compare August 11, 2025 20:59
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@tgross35 tgross35 added this pull request to the merge queue Aug 11, 2025
Merged via the queue into rust-lang:main with commit 8bd31d7 Aug 11, 2025
48 of 52 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Sep 19, 2025
(backport <rust-lang#4639>)
(cherry picked from commit 8bd31d7)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Sep 19, 2025
(backport <rust-lang#4639>)
(cherry picked from commit 8bd31d7)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Sep 19, 2025
(backport <rust-lang#4639>)
(cherry picked from commit 8bd31d7)
@tgross35 tgross35 mentioned this pull request Sep 19, 2025
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2025
(backport <#4639>)
(cherry picked from commit 8bd31d7)
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2025
(backport <#4639>)
(cherry picked from commit 8bd31d7)
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-linux-like O-unix stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

epoll_event should be packed on musl x86 too
3 participants