Skip to content

Commit

Permalink
Use libc specific type for architecture specific ioctl defines on Linux.
Browse files Browse the repository at this point in the history
  • Loading branch information
@de-vri-es
de-vri-es committed Nov 17, 2021
1 parent 83ffbee commit 7c95819
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 28 deletions.
9 changes: 9 additions & 0 deletions libc-test/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2740,6 +2740,9 @@ fn test_linux(target: &str) {
| "Elf64_Shdr" | "Elf32_Sym" | "Elf64_Sym" | "Elf32_Ehdr" | "Elf64_Ehdr"
| "Elf32_Chdr" | "Elf64_Chdr" => ty.to_string(),

"Ioctl" if gnu => "unsigned long".to_string(),
"Ioctl" => "int".to_string(),

t if is_union => format!("union {}", t),

t if t.ends_with("_t") => t.to_string(),
Expand Down Expand Up @@ -2797,6 +2800,9 @@ fn test_linux(target: &str) {
// on Linux, this is a volatile int
"pthread_spinlock_t" => true,

// For internal use only, to define architecture specific ioctl constants with a libc specific type.
"Ioctl" => true,

_ => false,
}
});
Expand Down Expand Up @@ -3227,6 +3233,7 @@ fn test_linux(target: &str) {
// This function tests APIs that are incompatible to test when other APIs
// are included (e.g. because including both sets of headers clashes)
fn test_linux_like_apis(target: &str) {
let gnu = target.contains("gnu");
let musl = target.contains("musl");
let linux = target.contains("linux");
let emscripten = target.contains("emscripten");
Expand Down Expand Up @@ -3293,6 +3300,8 @@ fn test_linux_like_apis(target: &str) {
})
.skip_struct(|s| s != "termios2")
.type_name(move |ty, is_struct, is_union| match ty {
"Ioctl" if gnu => "unsigned long".to_string(),
"Ioctl" => "int".to_string(),
t if is_struct => format!("struct {}", t),
t if is_union => format!("union {}", t),
t => t.to_string(),
Expand Down
16 changes: 8 additions & 8 deletions src/unix/linux_like/linux/arch/generic/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ cfg_if! {
pub const SCM_TIMESTAMPNS: ::c_int = SO_TIMESTAMPNS;
pub const SCM_TIMESTAMPING: ::c_int = SO_TIMESTAMPING;

pub const TIOCMGET: ::c_ulong = 0x5415;
pub const TIOCMBIS: ::c_ulong = 0x5416;
pub const TIOCMBIC: ::c_ulong = 0x5417;
pub const TIOCMSET: ::c_ulong = 0x5418;
pub const TCGETS2: ::c_ulong = 0x802c542a;
pub const TCSETS2: ::c_ulong = 0x402c542b;
pub const TCSETSW2: ::c_ulong = 0x402c542c;
pub const TCSETSF2: ::c_ulong = 0x402c542d;
pub const TIOCMGET: ::Ioctl = 0x5415;
pub const TIOCMBIS: ::Ioctl = 0x5416;
pub const TIOCMBIC: ::Ioctl = 0x5417;
pub const TIOCMSET: ::Ioctl = 0x5418;
pub const TCGETS2: ::Ioctl = 0x802c542a;
pub const TCSETS2: ::Ioctl = 0x402c542b;
pub const TCSETSW2: ::Ioctl = 0x402c542c;
pub const TCSETSF2: ::Ioctl = 0x402c542d;

pub const TIOCM_LE: ::c_int = 0x001;
pub const TIOCM_DTR: ::c_int = 0x002;
Expand Down
16 changes: 8 additions & 8 deletions src/unix/linux_like/linux/arch/mips/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ pub const SO_TIMESTAMPING: ::c_int = 37;
pub const SCM_TIMESTAMPNS: ::c_int = SO_TIMESTAMPNS;
pub const SCM_TIMESTAMPING: ::c_int = SO_TIMESTAMPING;

pub const TIOCMGET: ::c_ulong = 0x741d;
pub const TIOCMBIS: ::c_ulong = 0x741b;
pub const TIOCMBIC: ::c_ulong = 0x741c;
pub const TIOCMSET: ::c_ulong = 0x741a;
pub const TCGETS2: ::c_ulong = 0x4030542a;
pub const TCSETS2: ::c_ulong = 0x8030542b;
pub const TCSETSW2: ::c_ulong = 0x8030542c;
pub const TCSETSF2: ::c_ulong = 0x8030542d;
pub const TIOCMGET: ::Ioctl = 0x741d;
pub const TIOCMBIS: ::Ioctl = 0x741b;
pub const TIOCMBIC: ::Ioctl = 0x741c;
pub const TIOCMSET: ::Ioctl = 0x741a;
pub const TCGETS2: ::Ioctl = 0x4030542a;
pub const TCSETS2: ::Ioctl = 0x8030542b;
pub const TCSETSW2: ::Ioctl = 0x8030542c;
pub const TCSETSF2: ::Ioctl = 0x8030542d;

pub const TIOCM_LE: ::c_int = 0x001;
pub const TIOCM_DTR: ::c_int = 0x002;
Expand Down
8 changes: 4 additions & 4 deletions src/unix/linux_like/linux/arch/powerpc/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ pub const SO_BINDTOIFINDEX: ::c_int = 62;
pub const SCM_TIMESTAMPNS: ::c_int = SO_TIMESTAMPNS;
pub const SCM_TIMESTAMPING: ::c_int = SO_TIMESTAMPING;

pub const TIOCMGET: ::c_int = 0x5415;
pub const TIOCMBIS: ::c_int = 0x5416;
pub const TIOCMBIC: ::c_int = 0x5417;
pub const TIOCMSET: ::c_int = 0x5418;
pub const TIOCMGET: ::Ioctl = 0x5415;
pub const TIOCMBIS: ::Ioctl = 0x5416;
pub const TIOCMBIC: ::Ioctl = 0x5417;
pub const TIOCMSET: ::Ioctl = 0x5418;

pub const TIOCM_LE: ::c_int = 0x001;
pub const TIOCM_DTR: ::c_int = 0x002;
Expand Down
16 changes: 8 additions & 8 deletions src/unix/linux_like/linux/arch/sparc/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ pub const SO_TIMESTAMPING: ::c_int = 0x0023;
pub const SCM_TIMESTAMPNS: ::c_int = SO_TIMESTAMPNS;
pub const SCM_TIMESTAMPING: ::c_int = SO_TIMESTAMPING;

pub const TIOCMGET: ::c_ulong = 0x4004746a;
pub const TIOCMBIS: ::c_ulong = 0x8004746c;
pub const TIOCMBIC: ::c_ulong = 0x8004746b;
pub const TIOCMSET: ::c_ulong = 0x8004746d;
pub const TCGETS2: ::c_ulong = 0x402c540c;
pub const TCSETS2: ::c_ulong = 0x802c540d;
pub const TCSETSW2: ::c_ulong = 0x802c540e;
pub const TCSETSF2: ::c_ulong = 0x802c540f;
pub const TIOCMGET: ::Ioctl = 0x4004746a;
pub const TIOCMBIS: ::Ioctl = 0x8004746c;
pub const TIOCMBIC: ::Ioctl = 0x8004746b;
pub const TIOCMSET: ::Ioctl = 0x8004746d;
pub const TCGETS2: ::Ioctl = 0x402c540c;
pub const TCSETS2: ::Ioctl = 0x802c540d;
pub const TCSETSW2: ::Ioctl = 0x802c540e;
pub const TCSETSF2: ::Ioctl = 0x802c540f;

pub const TIOCM_LE: ::c_int = 0x001;
pub const TIOCM_DTR: ::c_int = 0x002;
Expand Down
10 changes: 10 additions & 0 deletions src/unix/linux_like/linux/gnu/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ pub type __rlimit_resource_t = ::c_uint;
pub type Lmid_t = ::c_long;
pub type regoff_t = ::c_int;

cfg_if! {
if #[cfg(doc)] {
// Used in `linux::arch` to define ioctl constants.
pub(crate) type Ioctl = ::c_ulong;
} else {
#[doc(hidden)]
pub type Ioctl = ::c_ulong;
}
}

s! {
pub struct statx {
pub stx_mask: u32,
Expand Down
10 changes: 10 additions & 0 deletions src/unix/linux_like/linux/musl/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@ pub type rlim_t = ::c_ulonglong;

pub type flock64 = flock;

cfg_if! {
if #[cfg(doc)] {
// Used in `linux::arch` to define ioctl constants.
pub(crate) type Ioctl = ::c_int;
} else {
#[doc(hidden)]
pub type Ioctl = ::c_int;
}
}

impl siginfo_t {
pub unsafe fn si_addr(&self) -> *mut ::c_void {
#[repr(C)]
Expand Down
10 changes: 10 additions & 0 deletions src/unix/linux_like/linux/uclibc/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ pub type regoff_t = ::c_int;
pub type __rlimit_resource_t = ::c_uint;
pub type __priority_which_t = ::c_uint;

cfg_if! {
if #[cfg(doc)] {
// Used in `linux::arch` to define ioctl constants.
pub(crate) type Ioctl = ::c_int;
} else {
#[doc(hidden)]
pub type Ioctl = ::c_int;

This comment has been minimized.

Sign in to view

Copy link
@skrap

skrap Jan 4, 2022 •
edited
Loading

Contributor

ship has sorta sailed on this one, but since ioctl is defined on uclibc as extern int ioctl (int __fd, unsigned long int __request, ...) __THROW;, should this be ::c_ulong instead? I can make the PR, if so.
(ref https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/include/sys/ioctl.h#n41 )

This comment has been minimized.

Sign in to view

Copy link
@de-vri-es

de-vri-es Jan 4, 2022 •
edited
Loading

Author Contributor

This was done to keep backwards compatibility with the previous versions of the libc crate. The mismatch is unfortunate, but not new with this PR I believe. Or is it?

This comment has been minimized.

Sign in to view

Copy link
@de-vri-es

de-vri-es Jan 4, 2022

Author Contributor

For example, these ioctl constants were already c_int:

libc/src/unix/linux_like/linux/uclibc/mod.rs

Lines 328 to 352 in 2910a53

pub const TCFLSH: ::c_int = 0x540B;
pub const TCGETA: ::c_int = 0x5405;
pub const TCGETS: ::c_int = 0x5401;
pub const TCP_COOKIE_TRANSACTIONS: ::c_int = 15;
pub const TCSBRK: ::c_int = 0x5409;
pub const TCSETA: ::c_int = 0x5406;
pub const TCSETAF: ::c_int = 0x5408;
pub const TCSETAW: ::c_int = 0x5407;
pub const TCSETS: ::c_int = 0x5402;
pub const TCSETSF: ::c_int = 0x5404;
pub const TCSETSW: ::c_int = 0x5403;
pub const TCXONC: ::c_int = 0x540A;
pub const TIOCCONS: ::c_int = 0x541D;
pub const TIOCEXCL: ::c_int = 0x540C;
pub const TIOCGPGRP: ::c_int = 0x540F;
pub const TIOCGSERIAL: ::c_int = 0x541E;
pub const TIOCGSOFTCAR: ::c_int = 0x5419;
pub const TIOCINQ: ::c_int = FIONREAD;
pub const TIOCLINUX: ::c_int = 0x541C;
pub const TIOCNXCL: ::c_int = 0x540D;
pub const TIOCOUTQ: ::c_int = 0x5411;
pub const TIOCSCTTY: ::c_int = 0x540E;
pub const TIOCSPGRP: ::c_int = 0x5410;
pub const TIOCSSOFTCAR: ::c_int = 0x541A;
pub const TIOCSTI: ::c_int = 0x5412;

Turning them all into c_ulong would be a breaking change :(

This comment has been minimized.

Sign in to view

Copy link
@skrap

skrap Jan 4, 2022 •
edited
Loading

Contributor

I've been slowly fixing up that arch, but I haven't had the need to look at termios-related functionality. PRs like #2615 make me wonder if it's always been broken in uclibc!

This comment has been minimized.

Sign in to view

Copy link
@skrap

skrap Jan 4, 2022

Contributor

Also I'd note that uclibc is tier-3 at best, so IMO backwards compatibility takes a back seat to correctness and cross-platform compatibility, at least right now.

This comment has been minimized.

Sign in to view

Copy link
@de-vri-es

de-vri-es Jan 4, 2022 •
edited
Loading

Author Contributor

Right. Whenever I do an ioctl I write something like ioctl(..., SOME_CONSTANT as _, ...) to avoid dealing with wrong integer types across platforms. Not super pretty, but it works and it's future proof :]

Anyway, I'm all for fixing the type, but I don't think my opinion on the matter matters much as I'm not a maintainer. Either way, you have my thanks for trying to clean it up :)
}
}

s! {
pub struct statvfs { // Different than GNU!
pub f_bsize: ::c_ulong,
Expand Down

0 comments on commit 7c95819

Please sign in to comment.