From 648a9db372c90e20090fae51563f68af205498fe Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 21 Jul 2017 00:16:03 +0200 Subject: [PATCH 1/2] Get rid of a lot of transmutes Most could be replaced by simple raw pointer casts (or even perfectly safe coercions!). cc #373 --- src/sched.rs | 2 +- src/sys/quota.rs | 10 ++-------- src/sys/signal.rs | 8 ++++---- src/sys/socket/addr.rs | 4 ++-- src/sys/socket/mod.rs | 2 +- src/sys/socket/sockopt.rs | 24 ++++++++++++------------ test/sys/test_socket.rs | 9 ++++++--- 7 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/sched.rs b/src/sched.rs index 0b98c5808a..9543e50d47 100644 --- a/src/sched.rs +++ b/src/sched.rs @@ -96,7 +96,7 @@ pub fn sched_setaffinity(pid: Pid, cpuset: &CpuSet) -> Result<()> { let res = unsafe { libc::sched_setaffinity(pid.into(), mem::size_of::() as libc::size_t, - mem::transmute(cpuset)) + &cpuset.cpu_set) }; Errno::result(res).map(drop) diff --git a/src/sys/quota.rs b/src/sys/quota.rs index b66d558d20..0b39fcf26a 100644 --- a/src/sys/quota.rs +++ b/src/sys/quota.rs @@ -111,16 +111,10 @@ pub fn quotactl_sync(which: quota::QuotaType, special: Opti } pub fn quotactl_get(which: quota::QuotaType, special: &P, id: c_int, dqblk: &mut quota::Dqblk) -> Result<()> { - use std::mem; - unsafe { - quotactl(quota::QuotaCmd(quota::Q_GETQUOTA, which), Some(special), id, mem::transmute(dqblk)) - } + quotactl(quota::QuotaCmd(quota::Q_GETQUOTA, which), Some(special), id, dqblk as *mut _ as *mut c_char) } pub fn quotactl_set(which: quota::QuotaType, special: &P, id: c_int, dqblk: "a::Dqblk) -> Result<()> { - use std::mem; let mut dqblk_copy = *dqblk; - unsafe { - quotactl(quota::QuotaCmd(quota::Q_SETQUOTA, which), Some(special), id, mem::transmute(&mut dqblk_copy)) - } + quotactl(quota::QuotaCmd(quota::Q_SETQUOTA, which), Some(special), id, &mut dqblk_copy as *mut _ as *mut c_char) } diff --git a/src/sys/signal.rs b/src/sys/signal.rs index d7e9d91d84..a5ec9e3c91 100644 --- a/src/sys/signal.rs +++ b/src/sys/signal.rs @@ -364,10 +364,10 @@ impl SigAction { pub fn new(handler: SigHandler, flags: SaFlags, mask: SigSet) -> SigAction { let mut s = unsafe { mem::uninitialized::() }; s.sa_sigaction = match handler { - SigHandler::SigDfl => unsafe { mem::transmute(libc::SIG_DFL) }, - SigHandler::SigIgn => unsafe { mem::transmute(libc::SIG_IGN) }, - SigHandler::Handler(f) => unsafe { mem::transmute(f) }, - SigHandler::SigAction(f) => unsafe { mem::transmute(f) }, + SigHandler::SigDfl => libc::SIG_DFL, + SigHandler::SigIgn => libc::SIG_IGN, + SigHandler::Handler(f) => f as *const extern fn(libc::c_int) as usize, + SigHandler::SigAction(f) => f as *const extern fn(libc::c_int, *mut libc::siginfo_t, *mut libc::c_void) as usize, }; s.sa_flags = match handler { SigHandler::SigAction(_) => (flags | SA_SIGINFO).bits(), diff --git a/src/sys/socket/addr.rs b/src/sys/socket/addr.rs index 71001534a6..d57207ed50 100644 --- a/src/sys/socket/addr.rs +++ b/src/sys/socket/addr.rs @@ -1,7 +1,7 @@ use super::sa_family_t; use {Errno, Error, Result, NixPath}; use libc; -use std::{fmt, hash, mem, net, ptr}; +use std::{fmt, hash, mem, net, ptr, slice}; use std::ffi::OsStr; use std::path::Path; use std::os::unix::ffi::OsStrExt; @@ -581,7 +581,7 @@ impl UnixAddr { } fn sun_path(&self) -> &[u8] { - unsafe { mem::transmute(&self.0.sun_path[..self.1]) } + unsafe { slice::from_raw_parts(self.0.sun_path.as_ptr() as *const u8, self.1) } } /// If this address represents a filesystem path, return that path. diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs index 6ab1684ad0..e0f957d8f6 100644 --- a/src/sys/socket/mod.rs +++ b/src/sys/socket/mod.rs @@ -227,7 +227,7 @@ impl<'a> Iterator for CmsgIterator<'a> { if self.buf.len() < sizeof_cmsghdr { return None; } - let cmsg: &cmsghdr = unsafe { mem::transmute(self.buf.as_ptr()) }; + let cmsg: &'a cmsghdr = unsafe { &*(self.buf.as_ptr() as *const cmsghdr) }; // This check is only in the glibc implementation of CMSG_NXTHDR // (although it claims the kernel header checks this), but such diff --git a/src/sys/socket/sockopt.rs b/src/sys/socket/sockopt.rs index 3c13c53968..1c6d0b859f 100644 --- a/src/sys/socket/sockopt.rs +++ b/src/sys/socket/sockopt.rs @@ -205,11 +205,11 @@ impl Get for GetStruct { } unsafe fn ffi_ptr(&mut self) -> *mut c_void { - mem::transmute(&mut self.val) + &mut self.val as *mut T as *mut c_void } unsafe fn ffi_len(&mut self) -> *mut socklen_t { - mem::transmute(&mut self.len) + &mut self.len } unsafe fn unwrap(self) -> T { @@ -228,7 +228,7 @@ impl<'a, T> Set<'a, T> for SetStruct<'a, T> { } unsafe fn ffi_ptr(&self) -> *const c_void { - mem::transmute(self.ptr) + self.ptr as *const T as *const c_void } unsafe fn ffi_len(&self) -> socklen_t { @@ -250,11 +250,11 @@ impl Get for GetBool { } unsafe fn ffi_ptr(&mut self) -> *mut c_void { - mem::transmute(&mut self.val) + &mut self.val as *mut c_int as *mut c_void } unsafe fn ffi_len(&mut self) -> *mut socklen_t { - mem::transmute(&mut self.len) + &mut self.len } unsafe fn unwrap(self) -> bool { @@ -273,7 +273,7 @@ impl<'a> Set<'a, bool> for SetBool { } unsafe fn ffi_ptr(&self) -> *const c_void { - mem::transmute(&self.val) + &self.val as *const c_int as *const c_void } unsafe fn ffi_len(&self) -> socklen_t { @@ -295,11 +295,11 @@ impl Get for GetU8 { } unsafe fn ffi_ptr(&mut self) -> *mut c_void { - mem::transmute(&mut self.val) + &mut self.val as *mut uint8_t as *mut c_void } unsafe fn ffi_len(&mut self) -> *mut socklen_t { - mem::transmute(&mut self.len) + &mut self.len } unsafe fn unwrap(self) -> u8 { @@ -318,7 +318,7 @@ impl<'a> Set<'a, u8> for SetU8 { } unsafe fn ffi_ptr(&self) -> *const c_void { - mem::transmute(&self.val) + &self.val as *const uint8_t as *const c_void } unsafe fn ffi_len(&self) -> socklen_t { @@ -340,11 +340,11 @@ impl Get for GetUsize { } unsafe fn ffi_ptr(&mut self) -> *mut c_void { - mem::transmute(&mut self.val) + &mut self.val as *mut c_int as *mut c_void } unsafe fn ffi_len(&mut self) -> *mut socklen_t { - mem::transmute(&mut self.len) + &mut self.len } unsafe fn unwrap(self) -> usize { @@ -363,7 +363,7 @@ impl<'a> Set<'a, usize> for SetUsize { } unsafe fn ffi_ptr(&self) -> *const c_void { - mem::transmute(&self.val) + &self.val as *const c_int as *const c_void } unsafe fn ffi_len(&self) -> socklen_t { diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index 31b153ddbe..b8dc662eb9 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -1,5 +1,5 @@ use nix::sys::socket::{InetAddr, UnixAddr, getsockname}; -use std::mem; +use std::{mem, slice}; use std::net::{self, Ipv6Addr, SocketAddr, SocketAddrV6}; use std::path::Path; use std::str::FromStr; @@ -52,10 +52,13 @@ pub fn test_inetv6_addr_to_sock_addr() { #[test] pub fn test_path_to_sock_addr() { - let actual = Path::new("/foo/bar"); + let path = "/foo/bar"; + let actual = Path::new(path); let addr = UnixAddr::new(actual).unwrap(); - let expect: &'static [c_char] = unsafe { mem::transmute(&b"/foo/bar"[..]) }; + let expect: &[c_char] = unsafe { + slice::from_raw_parts(path.as_bytes().as_ptr() as *const c_char, path.len()) + }; assert_eq!(&addr.0.sun_path[..8], expect); assert_eq!(addr.path(), Some(actual)); From 9c9af2c6638c9a2c91fb18ff3cb79c4bac626965 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 25 Jul 2017 19:04:38 +0200 Subject: [PATCH 2/2] Fix safety of sockopt helper traits --- src/sys/socket/sockopt.rs | 60 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/sys/socket/sockopt.rs b/src/sys/socket/sockopt.rs index 1c6d0b859f..7e7812309a 100644 --- a/src/sys/socket/sockopt.rs +++ b/src/sys/socket/sockopt.rs @@ -178,17 +178,17 @@ sockopt_impl!(GetOnly, OriginalDst, libc::SOL_IP, libc::SO_ORIGINAL_DST, libc::s * */ -trait Get { +unsafe trait Get { unsafe fn blank() -> Self; - unsafe fn ffi_ptr(&mut self) -> *mut c_void; - unsafe fn ffi_len(&mut self) -> *mut socklen_t; + fn ffi_ptr(&mut self) -> *mut c_void; + fn ffi_len(&mut self) -> *mut socklen_t; unsafe fn unwrap(self) -> T; } -trait Set<'a, T> { +unsafe trait Set<'a, T> { fn new(val: &'a T) -> Self; - unsafe fn ffi_ptr(&self) -> *const c_void; - unsafe fn ffi_len(&self) -> socklen_t; + fn ffi_ptr(&self) -> *const c_void; + fn ffi_len(&self) -> socklen_t; } struct GetStruct { @@ -196,7 +196,7 @@ struct GetStruct { val: T, } -impl Get for GetStruct { +unsafe impl Get for GetStruct { unsafe fn blank() -> Self { GetStruct { len: mem::size_of::() as socklen_t, @@ -204,11 +204,11 @@ impl Get for GetStruct { } } - unsafe fn ffi_ptr(&mut self) -> *mut c_void { + fn ffi_ptr(&mut self) -> *mut c_void { &mut self.val as *mut T as *mut c_void } - unsafe fn ffi_len(&mut self) -> *mut socklen_t { + fn ffi_len(&mut self) -> *mut socklen_t { &mut self.len } @@ -222,16 +222,16 @@ struct SetStruct<'a, T: 'static> { ptr: &'a T, } -impl<'a, T> Set<'a, T> for SetStruct<'a, T> { +unsafe impl<'a, T> Set<'a, T> for SetStruct<'a, T> { fn new(ptr: &'a T) -> SetStruct<'a, T> { SetStruct { ptr: ptr } } - unsafe fn ffi_ptr(&self) -> *const c_void { + fn ffi_ptr(&self) -> *const c_void { self.ptr as *const T as *const c_void } - unsafe fn ffi_len(&self) -> socklen_t { + fn ffi_len(&self) -> socklen_t { mem::size_of::() as socklen_t } } @@ -241,7 +241,7 @@ struct GetBool { val: c_int, } -impl Get for GetBool { +unsafe impl Get for GetBool { unsafe fn blank() -> Self { GetBool { len: mem::size_of::() as socklen_t, @@ -249,11 +249,11 @@ impl Get for GetBool { } } - unsafe fn ffi_ptr(&mut self) -> *mut c_void { + fn ffi_ptr(&mut self) -> *mut c_void { &mut self.val as *mut c_int as *mut c_void } - unsafe fn ffi_len(&mut self) -> *mut socklen_t { + fn ffi_len(&mut self) -> *mut socklen_t { &mut self.len } @@ -267,16 +267,16 @@ struct SetBool { val: c_int, } -impl<'a> Set<'a, bool> for SetBool { +unsafe impl<'a> Set<'a, bool> for SetBool { fn new(val: &'a bool) -> SetBool { SetBool { val: if *val { 1 } else { 0 } } } - unsafe fn ffi_ptr(&self) -> *const c_void { + fn ffi_ptr(&self) -> *const c_void { &self.val as *const c_int as *const c_void } - unsafe fn ffi_len(&self) -> socklen_t { + fn ffi_len(&self) -> socklen_t { mem::size_of::() as socklen_t } } @@ -286,7 +286,7 @@ struct GetU8 { val: uint8_t, } -impl Get for GetU8 { +unsafe impl Get for GetU8 { unsafe fn blank() -> Self { GetU8 { len: mem::size_of::() as socklen_t, @@ -294,11 +294,11 @@ impl Get for GetU8 { } } - unsafe fn ffi_ptr(&mut self) -> *mut c_void { + fn ffi_ptr(&mut self) -> *mut c_void { &mut self.val as *mut uint8_t as *mut c_void } - unsafe fn ffi_len(&mut self) -> *mut socklen_t { + fn ffi_len(&mut self) -> *mut socklen_t { &mut self.len } @@ -312,16 +312,16 @@ struct SetU8 { val: uint8_t, } -impl<'a> Set<'a, u8> for SetU8 { +unsafe impl<'a> Set<'a, u8> for SetU8 { fn new(val: &'a u8) -> SetU8 { SetU8 { val: *val as uint8_t } } - unsafe fn ffi_ptr(&self) -> *const c_void { + fn ffi_ptr(&self) -> *const c_void { &self.val as *const uint8_t as *const c_void } - unsafe fn ffi_len(&self) -> socklen_t { + fn ffi_len(&self) -> socklen_t { mem::size_of::() as socklen_t } } @@ -331,7 +331,7 @@ struct GetUsize { val: c_int, } -impl Get for GetUsize { +unsafe impl Get for GetUsize { unsafe fn blank() -> Self { GetUsize { len: mem::size_of::() as socklen_t, @@ -339,11 +339,11 @@ impl Get for GetUsize { } } - unsafe fn ffi_ptr(&mut self) -> *mut c_void { + fn ffi_ptr(&mut self) -> *mut c_void { &mut self.val as *mut c_int as *mut c_void } - unsafe fn ffi_len(&mut self) -> *mut socklen_t { + fn ffi_len(&mut self) -> *mut socklen_t { &mut self.len } @@ -357,16 +357,16 @@ struct SetUsize { val: c_int, } -impl<'a> Set<'a, usize> for SetUsize { +unsafe impl<'a> Set<'a, usize> for SetUsize { fn new(val: &'a usize) -> SetUsize { SetUsize { val: *val as c_int } } - unsafe fn ffi_ptr(&self) -> *const c_void { + fn ffi_ptr(&self) -> *const c_void { &self.val as *const c_int as *const c_void } - unsafe fn ffi_len(&self) -> socklen_t { + fn ffi_len(&self) -> socklen_t { mem::size_of::() as socklen_t } }