Skip to content

Commit

Permalink
Replace most instances of mem::uninitialized with mem::MaybeUninit
Browse files Browse the repository at this point in the history
Only two instances remain:

* For the deprecated sys::socket::CmsgSpace::new.  We should probably
  just remove that method.

* For sys::termios::Termios::default_uninit.  This will require some
  more thought.

Fixes #1096
  • Loading branch information
asomers committed Sep 3, 2019
1 parent a4a465d commit 4690324
Show file tree
Hide file tree
Showing 21 changed files with 236 additions and 196 deletions.
9 changes: 4 additions & 5 deletions CONVENTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,11 @@ to parameters of functions by [enumerations][enum].

Whenever we need to use a [libc][libc] function to properly initialize a
variable and said function allows us to use uninitialized memory, we use
[`std::mem::uninitialized`][std_uninitialized] (or [`core::mem::uninitialized`][core_uninitialized])
when defining the variable. This allows us to avoid the overhead incurred by
zeroing or otherwise initializing the variable.
[`std::mem::MaybeUninit`][std_MaybeUninit] when defining the variable. This
allows us to avoid the overhead incurred by zeroing or otherwise initializing
the variable.

[bitflags]: https://crates.io/crates/bitflags/
[core_uninitialized]: https://doc.rust-lang.org/core/mem/fn.uninitialized.html
[enum]: https://doc.rust-lang.org/reference.html#enumerations
[libc]: https://crates.io/crates/libc/
[std_uninitialized]: https://doc.rust-lang.org/std/mem/fn.uninitialized.html
[std_MaybeUninit]: https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html
11 changes: 7 additions & 4 deletions src/dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,18 @@ impl<'d> Iterator for Iter<'d> {
// for the NUL byte. It doesn't look like the std library does this; it just uses
// fixed-sized buffers (and libc's dirent seems to be sized so this is appropriate).
// Probably fine here too then.
let mut ent: Entry = Entry(::std::mem::uninitialized());
let mut ent = std::mem::MaybeUninit::<dirent>::uninit();
let mut result = ptr::null_mut();
if let Err(e) = Errno::result(readdir_r((self.0).0.as_ptr(), &mut ent.0, &mut result)) {
if let Err(e) = Errno::result(
readdir_r((self.0).0.as_ptr(), ent.as_mut_ptr(), &mut result))
{
return Some(Err(e));
}
if result.is_null() {
return None;
}
assert_eq!(result, &mut ent.0 as *mut dirent);
Some(Ok(ent))
assert_eq!(result, ent.as_mut_ptr());
Some(Ok(Entry(ent.assume_init())))
}
}
}
Expand All @@ -126,6 +128,7 @@ impl<'d> Drop for Iter<'d> {
///
/// Note that unlike the std version, this may represent the `.` or `..` entries.
#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
#[repr(transparent)]
pub struct Entry(dirent);

#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
Expand Down
16 changes: 9 additions & 7 deletions src/ifaddrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,15 @@ impl Iterator for InterfaceAddressIterator {
/// }
/// ```
pub fn getifaddrs() -> Result<InterfaceAddressIterator> {
let mut addrs: *mut libc::ifaddrs = unsafe { mem::uninitialized() };
Errno::result(unsafe { libc::getifaddrs(&mut addrs) }).map(|_| {
InterfaceAddressIterator {
base: addrs,
next: addrs,
}
})
let mut addrs = mem::MaybeUninit::<*mut libc::ifaddrs>::uninit();
unsafe {
Errno::result(libc::getifaddrs(addrs.as_mut_ptr())).map(|_| {
InterfaceAddressIterator {
base: addrs.assume_init(),
next: addrs.assume_init(),
}
})
}
}

#[cfg(test)]
Expand Down
32 changes: 19 additions & 13 deletions src/mqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,17 @@ impl MqAttr {
mq_maxmsg: c_long,
mq_msgsize: c_long,
mq_curmsgs: c_long)
-> MqAttr {
let mut attr = unsafe { mem::uninitialized::<libc::mq_attr>() };
attr.mq_flags = mq_flags;
attr.mq_maxmsg = mq_maxmsg;
attr.mq_msgsize = mq_msgsize;
attr.mq_curmsgs = mq_curmsgs;
MqAttr { mq_attr: attr }
-> MqAttr
{
let mut attr = mem::MaybeUninit::<libc::mq_attr>::uninit();
unsafe {
let p = attr.as_mut_ptr();
(*p).mq_flags = mq_flags;
(*p).mq_maxmsg = mq_maxmsg;
(*p).mq_msgsize = mq_msgsize;
(*p).mq_curmsgs = mq_curmsgs;
MqAttr { mq_attr: attr.assume_init() }
}
}

pub fn flags(&self) -> c_long {
Expand Down Expand Up @@ -123,9 +127,9 @@ pub fn mq_send(mqdes: mqd_t, message: &[u8], msq_prio: u32) -> Result<()> {
///
/// See also [`mq_getattr(2)`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/mq_getattr.html)
pub fn mq_getattr(mqd: mqd_t) -> Result<MqAttr> {
let mut attr = unsafe { mem::uninitialized::<libc::mq_attr>() };
let res = unsafe { libc::mq_getattr(mqd, &mut attr) };
Errno::result(res).map(|_| MqAttr { mq_attr: attr })
let mut attr = mem::MaybeUninit::<libc::mq_attr>::uninit();
let res = unsafe { libc::mq_getattr(mqd, attr.as_mut_ptr()) };
Errno::result(res).map(|_| unsafe{MqAttr { mq_attr: attr.assume_init() }})
}

/// Set the attributes of the message queue. Only `O_NONBLOCK` can be set, everything else will be ignored
Expand All @@ -134,9 +138,11 @@ pub fn mq_getattr(mqd: mqd_t) -> Result<MqAttr> {
///
/// [Further reading](http://pubs.opengroup.org/onlinepubs/9699919799/functions/mq_setattr.html)
pub fn mq_setattr(mqd: mqd_t, newattr: &MqAttr) -> Result<MqAttr> {
let mut attr = unsafe { mem::uninitialized::<libc::mq_attr>() };
let res = unsafe { libc::mq_setattr(mqd, &newattr.mq_attr as *const libc::mq_attr, &mut attr) };
Errno::result(res).map(|_| MqAttr { mq_attr: attr })
let mut attr = mem::MaybeUninit::<libc::mq_attr>::uninit();
let res = unsafe {
libc::mq_setattr(mqd, &newattr.mq_attr as *const libc::mq_attr, attr.as_mut_ptr())
};
Errno::result(res).map(|_| unsafe{ MqAttr { mq_attr: attr.assume_init() }})
}

/// Convenience function.
Expand Down
44 changes: 24 additions & 20 deletions src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,16 @@ pub fn unlockpt(fd: &PtyMaster) -> Result<()> {
pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>>>(winsize: T, termios: U) -> Result<OpenptyResult> {
use std::ptr;

let mut slave: libc::c_int = unsafe { mem::uninitialized() };
let mut master: libc::c_int = unsafe { mem::uninitialized() };
let mut slave = mem::MaybeUninit::<libc::c_int>::uninit();
let mut master = mem::MaybeUninit::<libc::c_int>::uninit();
let ret = {
match (termios.into(), winsize.into()) {
(Some(termios), Some(winsize)) => {
let inner_termios = termios.get_libc_termios();
unsafe {
libc::openpty(
&mut master,
&mut slave,
master.as_mut_ptr(),
slave.as_mut_ptr(),
ptr::null_mut(),
&*inner_termios as *const libc::termios as *mut _,
winsize as *const Winsize as *mut _,
Expand All @@ -237,8 +237,8 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
(None, Some(winsize)) => {
unsafe {
libc::openpty(
&mut master,
&mut slave,
master.as_mut_ptr(),
slave.as_mut_ptr(),
ptr::null_mut(),
ptr::null_mut(),
winsize as *const Winsize as *mut _,
Expand All @@ -249,8 +249,8 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
let inner_termios = termios.get_libc_termios();
unsafe {
libc::openpty(
&mut master,
&mut slave,
master.as_mut_ptr(),
slave.as_mut_ptr(),
ptr::null_mut(),
&*inner_termios as *const libc::termios as *mut _,
ptr::null_mut(),
Expand All @@ -260,8 +260,8 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
(None, None) => {
unsafe {
libc::openpty(
&mut master,
&mut slave,
master.as_mut_ptr(),
slave.as_mut_ptr(),
ptr::null_mut(),
ptr::null_mut(),
ptr::null_mut(),
Expand All @@ -273,10 +273,12 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>

Errno::result(ret)?;

Ok(OpenptyResult {
master,
slave,
})
unsafe {
Ok(OpenptyResult {
master: master.assume_init(),
slave: slave.assume_init(),
})
}
}

/// Create a new pseudoterminal, returning the master file descriptor and forked pid.
Expand All @@ -294,7 +296,7 @@ pub fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
use unistd::Pid;
use unistd::ForkResult::*;

let mut master: libc::c_int = unsafe { mem::uninitialized() };
let mut master = mem::MaybeUninit::<libc::c_int>::uninit();

let term = match termios.into() {
Some(termios) => {
Expand All @@ -310,17 +312,19 @@ pub fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
.unwrap_or(ptr::null_mut());

let res = unsafe {
libc::forkpty(&mut master, ptr::null_mut(), term, win)
libc::forkpty(master.as_mut_ptr(), ptr::null_mut(), term, win)
};

let fork_result = Errno::result(res).map(|res| match res {
0 => Child,
res => Parent { child: Pid::from_raw(res) },
})?;

Ok(ForkptyResult {
master,
fork_result,
})
unsafe {
Ok(ForkptyResult {
master: master.assume_init(),
fork_result,
})
}
}

7 changes: 3 additions & 4 deletions src/sys/ptrace/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,16 +221,15 @@ pub fn setregs(pid: Pid, regs: user_regs_struct) -> Result<()> {
/// and therefore use the data field to return values. This function handles these
/// requests.
fn ptrace_get_data<T>(request: Request, pid: Pid) -> Result<T> {
// Creates an uninitialized pointer to store result in
let data: T = unsafe { mem::uninitialized() };
let mut data = mem::MaybeUninit::uninit();
let res = unsafe {
libc::ptrace(request as RequestType,
libc::pid_t::from(pid),
ptr::null_mut::<T>(),
&data as *const _ as *const c_void)
data.as_mut_ptr() as *const _ as *const c_void)
};
Errno::result(res)?;
Ok(data)
Ok(unsafe{ data.assume_init() })
}

unsafe fn ptrace_other(request: Request, pid: Pid, addr: AddressType, data: *mut c_void) -> Result<c_long> {
Expand Down
9 changes: 4 additions & 5 deletions src/sys/quota.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ libc_bitflags!(
);

/// Wrapper type for `if_dqblk`
// FIXME: Change to repr(transparent)
#[repr(C)]
#[repr(transparent)]
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct Dqblk(libc::dqblk);

Expand Down Expand Up @@ -260,9 +259,9 @@ pub fn quotactl_sync<P: ?Sized + NixPath>(which: QuotaType, special: Option<&P>)

/// Get disk quota limits and current usage for the given user/group id.
pub fn quotactl_get<P: ?Sized + NixPath>(which: QuotaType, special: &P, id: c_int) -> Result<Dqblk> {
let mut dqblk = unsafe { mem::uninitialized() };
quotactl(QuotaCmd(QuotaSubCmd::Q_GETQUOTA, which), Some(special), id, &mut dqblk as *mut _ as *mut c_char)?;
dqblk
let mut dqblk = mem::MaybeUninit::uninit();
quotactl(QuotaCmd(QuotaSubCmd::Q_GETQUOTA, which), Some(special), id, dqblk.as_mut_ptr() as *mut c_char)?;
Ok(unsafe{ Dqblk(dqblk.assume_init())})
}

/// Configure quota values for the specified fields for a given user/group id.
Expand Down
11 changes: 6 additions & 5 deletions src/sys/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ use sys::time::{TimeSpec, TimeVal};

pub use libc::FD_SETSIZE;

// FIXME: Change to repr(transparent) once it's stable
#[repr(C)]
#[repr(transparent)]
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct FdSet(libc::fd_set);

impl FdSet {
pub fn new() -> FdSet {
let mut fdset = unsafe { mem::uninitialized() };
unsafe { libc::FD_ZERO(&mut fdset) };
FdSet(fdset)
let mut fdset = mem::MaybeUninit::uninit();
unsafe {
libc::FD_ZERO(fdset.as_mut_ptr());
FdSet(fdset.assume_init())
}
}

pub fn insert(&mut self, fd: RawFd) {
Expand Down
Loading

0 comments on commit 4690324

Please sign in to comment.