Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Linux: add getitimer()/setitimer() #3847

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Linux: add getitimer()/setitimer() #3847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions libc-test/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3598,6 +3598,9 @@ fn test_linux(target: &str) {
// https://github.com/rust-lang/libc/issues/1359
"sighandler_t" => true,

// musl doesn't define these; instead, it uses a raw int for getitimer/setitimer
"__itimer_which_t" if musl => true,

// These cannot be tested when "resolv.h" is included and are tested
// in the `linux_elf.rs` file.
"Elf64_Phdr" | "Elf32_Phdr" => true,
Expand Down
3 changes: 3 additions & 0 deletions libc-test/semver/linux.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3478,6 +3478,7 @@ __c_anonymous_sockaddr_can_j1939
__c_anonymous_sockaddr_can_tp
__errno_location
__exit_status
__itimer_which_t
__s16
__s32
__u16
Expand Down Expand Up @@ -3594,6 +3595,7 @@ getgrnam_r
getgrouplist
gethostid
getifaddrs
getitimer
getline
getmntent
getnameinfo
Expand Down Expand Up @@ -3895,6 +3897,7 @@ setfsuid
setgrent
setgroups
sethostname
setitimer
setmntent
setns
setpriority
Expand Down
30 changes: 30 additions & 0 deletions src/unix/linux_like/linux/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ missing! {
pub enum fpos64_t {} // FIXME: fill this out with a struct
}

cfg_if! {
if #[cfg(target_env = "musl")] {
pub type __itimer_which_t = ::c_int;
} else {
pub type __itimer_which_t = ::c_uint;
}
}
Comment on lines +76 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

glibc has int when it is not defined as an enum, and musl doesn't seem to define this type at all - why is the definition done this way?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah nevermind, I see the reasoning below. I think we need to work with one of the ways that doesn't expose this on musl.

In general it is probably better to avoid Rust enums to represent C types at all, given how easy it is to cause UB. So some sort of solution that just typedefs an int on glibc seems preferable to me.


e! {
pub enum tpacket_versions {
TPACKET_V1,
Expand Down Expand Up @@ -6116,6 +6124,28 @@ extern "C" {
pub fn ioctl(fd: ::c_int, request: ::Ioctl, ...) -> ::c_int;
}

cfg_if! {
if #[cfg(target_env = "musl")] {
extern "C" {
pub fn getitimer(which: ::c_int, value: *mut ::itimerval) -> ::c_int;
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure I follow, why not using the typedef you created above ?

Copy link
Contributor Author

@nathaniel-bennett nathaniel-bennett Aug 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is the result of a strange combination of issues. glibc and most other libc implementations typedef __itimer_which_t as an enum when _GNU_SOURCE is set, or as an int otherwise. musl unfortunately opts to just use an int directly within the function definitions for getitimer and setitimer. As a result of this:

  • Using __itimer_whiich_t directly in function definitions causes musl CI to fail as it's not a defined type
  • Removing __itimer_which_t altogether for musl builds still causes CI to fail, as there is currently no way of opting out of enum semver checks (you can opt out for structs, types, everything else but enums)

By defining __itimer_which_t as a type definition in musl and then opting out of it, CI passes.

A more clean solution to this may be to develop enum opt-out functions to the ctest2 crate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another possible approach is not to bother with enums at all.

pub type __itimer_which_t = ::c_int;
...
pub const ITIMER_REAL: __itimer_which_t = 0;
...

There are various places in this crate were C enums are handled as simple constants. Should pass in theory.

pub fn setitimer(
which: ::c_int,
new: *const ::itimerval,
old: *mut ::itimerval,
) -> ::c_int;
}
} else {
extern "C" {
pub fn getitimer(which: ::__itimer_which_t, value: *mut ::itimerval) -> ::c_int;
pub fn setitimer(
which: ::__itimer_which_t,
new: *const ::itimerval,
old: *mut ::itimerval,
) -> ::c_int;
}
}
}

// LFS64 extensions
//
// * musl has 64-bit versions only so aliases the LFS64 symbols to the standard ones
Expand Down