Replace std::mem::uninitialized with MaybeUninit (#1027)

* Replace `std::mem::uninitialized` with `MaybeUninit`

* Avoid undefined behavior when using `MaybeUninit`

* Restore unused `PointerState` fields as internally public

* Zero-initialize some struct values in Xlib FFI calls

* Reform usage of `MaybeUninit` in Xlib FFI

* Prefer safe zero-initialization using `Default`, when possible
* Zero-initialize integers and floats using `0` or `0.0`
* Use `MaybeUninit::uninit` for large byte buffers and union types
* Use `MaybeUninit::uninit` when the resulting value is ignored
This commit is contained in:
Murarth 2019-07-11 09:34:32 -07:00 committed by Hal Gentz
parent 17b8310517
commit 7daf146801
9 changed files with 186 additions and 126 deletions

View file

@ -1,6 +1,6 @@
#![cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd", target_os = "netbsd", target_os = "openbsd"))]
use std::{collections::VecDeque, env, ffi::CStr, fmt, mem, os::raw::*, sync::Arc};
use std::{collections::VecDeque, env, ffi::CStr, fmt, mem::MaybeUninit, os::raw::*, sync::Arc};
use parking_lot::Mutex;
use smithay_client_toolkit::reexports::client::ConnectError;
@ -410,14 +410,16 @@ unsafe extern "C" fn x_error_callback(
) -> c_int {
let xconn_lock = X11_BACKEND.lock();
if let Ok(ref xconn) = *xconn_lock {
let mut buf: [c_char; 1024] = mem::uninitialized();
// `assume_init` is safe here because the array consists of `MaybeUninit` values,
// which do not require initialization.
let mut buf: [MaybeUninit<c_char>; 1024] = MaybeUninit::uninit().assume_init();
(xconn.xlib.XGetErrorText)(
display,
(*event).error_code as c_int,
buf.as_mut_ptr(),
buf.as_mut_ptr() as *mut c_char,
buf.len() as c_int,
);
let description = CStr::from_ptr(buf.as_ptr()).to_string_lossy();
let description = CStr::from_ptr(buf.as_ptr() as *const c_char).to_string_lossy();
let error = XError {
description: description.into_owned(),

View file

@ -20,7 +20,7 @@ use std::{
cell::RefCell,
collections::{HashMap, HashSet, VecDeque},
ffi::CStr,
mem,
mem::{self, MaybeUninit},
ops::Deref,
os::raw::*,
rc::Rc,
@ -100,22 +100,21 @@ impl<T: 'static> EventLoop<T> {
.expect("Failed to query XRandR extension");
let xi2ext = unsafe {
let mut result = XExtension {
opcode: mem::uninitialized(),
first_event_id: mem::uninitialized(),
first_error_id: mem::uninitialized(),
};
let mut ext = XExtension::default();
let res = (xconn.xlib.XQueryExtension)(
xconn.display,
b"XInputExtension\0".as_ptr() as *const c_char,
&mut result.opcode as *mut c_int,
&mut result.first_event_id as *mut c_int,
&mut result.first_error_id as *mut c_int,
&mut ext.opcode,
&mut ext.first_event_id,
&mut ext.first_error_id,
);
if res == ffi::False {
panic!("X server missing XInput extension");
}
result
ext
};
unsafe {
@ -204,8 +203,9 @@ impl<T: 'static> EventLoop<T> {
move |evt, &mut ()| {
if evt.readiness.is_readable() {
// process all pending events
let mut xev = unsafe { mem::uninitialized() };
while unsafe { processor.poll_one_event(&mut xev) } {
let mut xev = MaybeUninit::uninit();
while unsafe { processor.poll_one_event(xev.as_mut_ptr()) } {
let mut xev = unsafe { xev.assume_init() };
processor.process_event(&mut xev, &mut callback);
}
}
@ -401,9 +401,10 @@ struct DeviceInfo<'a> {
impl<'a> DeviceInfo<'a> {
fn get(xconn: &'a XConnection, device: c_int) -> Option<Self> {
unsafe {
let mut count = mem::uninitialized();
let mut count = 0;
let info = (xconn.xinput2.XIQueryDevice)(xconn.display, device, &mut count);
xconn.check_errors().ok().and_then(|_| {
xconn.check_errors().ok()?;
if info.is_null() || count == 0 {
None
} else {
@ -413,7 +414,6 @@ impl<'a> DeviceInfo<'a> {
count: count as usize,
})
}
})
}
}
}
@ -518,7 +518,7 @@ impl<'a> Drop for GenericEventCookie<'a> {
}
}
#[derive(Debug, Copy, Clone)]
#[derive(Debug, Default, Copy, Clone)]
struct XExtension {
opcode: c_int,
first_event_id: c_int,

View file

@ -30,13 +30,17 @@ impl XConnection {
event_mask: Option<c_long>,
data: ClientMsgPayload,
) -> Flusher<'_> {
let mut event: ffi::XClientMessageEvent = unsafe { mem::uninitialized() };
event.type_ = ffi::ClientMessage;
event.display = self.display;
event.window = window;
event.message_type = message_type;
event.format = c_long::FORMAT as c_int;
event.data = unsafe { mem::transmute(data) };
let event = ffi::XClientMessageEvent {
type_: ffi::ClientMessage,
display: self.display,
window,
message_type,
format: c_long::FORMAT as c_int,
data: unsafe { mem::transmute(data) },
// These fields are ignored by `XSendEvent`
serial: 0,
send_event: 0,
};
self.send_event(target_window, event_mask, event)
}
@ -54,12 +58,17 @@ impl XConnection {
let format = T::FORMAT;
let size_of_t = mem::size_of::<T>();
debug_assert_eq!(size_of_t, format.get_actual_size());
let mut event: ffi::XClientMessageEvent = unsafe { mem::uninitialized() };
event.type_ = ffi::ClientMessage;
event.display = self.display;
event.window = window;
event.message_type = message_type;
event.format = format as c_int;
let mut event = ffi::XClientMessageEvent {
type_: ffi::ClientMessage,
display: self.display,
window,
message_type,
format: format as c_int,
data: ffi::ClientMessageData::new(),
// These fields are ignored by `XSendEvent`
serial: 0,
send_event: 0,
};
let t_per_payload = format.get_payload_size() / size_of_t;
assert!(t_per_payload > 0);

View file

@ -41,14 +41,14 @@ impl AaRect {
}
}
#[derive(Debug)]
#[derive(Debug, Default)]
pub struct TranslatedCoords {
pub x_rel_root: c_int,
pub y_rel_root: c_int,
pub child: ffi::Window,
}
#[derive(Debug)]
#[derive(Debug, Default)]
pub struct Geometry {
pub root: ffi::Window,
// If you want positions relative to the root window, use translate_coords.
@ -183,7 +183,8 @@ impl XConnection {
window: ffi::Window,
root: ffi::Window,
) -> Result<TranslatedCoords, XError> {
let mut translated_coords: TranslatedCoords = unsafe { mem::uninitialized() };
let mut coords = TranslatedCoords::default();
unsafe {
(self.xlib.XTranslateCoordinates)(
self.display,
@ -191,18 +192,20 @@ impl XConnection {
root,
0,
0,
&mut translated_coords.x_rel_root,
&mut translated_coords.y_rel_root,
&mut translated_coords.child,
&mut coords.x_rel_root,
&mut coords.y_rel_root,
&mut coords.child,
);
}
//println!("XTranslateCoordinates coords:{:?}", translated_coords);
self.check_errors().map(|_| translated_coords)
self.check_errors()?;
Ok(coords)
}
// This is adequate for inner_size
pub fn get_geometry(&self, window: ffi::Window) -> Result<Geometry, XError> {
let mut geometry: Geometry = unsafe { mem::uninitialized() };
let mut geometry = Geometry::default();
let _status = unsafe {
(self.xlib.XGetGeometry)(
self.display,
@ -216,8 +219,9 @@ impl XConnection {
&mut geometry.depth,
)
};
//println!("XGetGeometry geo:{:?}", geometry);
self.check_errors().map(|_| geometry)
self.check_errors()?;
Ok(geometry)
}
fn get_frame_extents(&self, window: ffi::Window) -> Option<FrameExtents> {
@ -264,10 +268,10 @@ impl XConnection {
fn get_parent_window(&self, window: ffi::Window) -> Result<ffi::Window, XError> {
let parent = unsafe {
let mut root: ffi::Window = mem::uninitialized();
let mut parent: ffi::Window = mem::uninitialized();
let mut root = 0;
let mut parent = 0;
let mut children: *mut ffi::Window = ptr::null_mut();
let mut nchildren: c_uint = mem::uninitialized();
let mut nchildren = 0;
// What's filled into `parent` if `window` is the root window?
let _status = (self.xlib.XQueryTree)(

View file

@ -317,13 +317,13 @@ impl XConnection {
pub fn get_normal_hints(&self, window: ffi::Window) -> Result<NormalHints<'_>, XError> {
let size_hints = self.alloc_size_hints();
let mut supplied_by_user: c_long = unsafe { mem::uninitialized() };
let mut supplied_by_user = MaybeUninit::uninit();
unsafe {
(self.xlib.XGetWMNormalHints)(
self.display,
window,
size_hints.ptr,
&mut supplied_by_user,
supplied_by_user.as_mut_ptr(),
);
}
self.check_errors().map(|_| NormalHints { size_hints })

View file

@ -1,4 +1,4 @@
use std::str;
use std::{slice, str};
use super::*;
use crate::event::ModifiersState;
@ -23,18 +23,19 @@ impl From<ffi::XIModifierState> for ModifiersState {
}
}
// NOTE: Some of these fields are not used, but may be of use in the future.
pub struct PointerState<'a> {
xconn: &'a XConnection,
root: ffi::Window,
child: ffi::Window,
pub root: ffi::Window,
pub child: ffi::Window,
pub root_x: c_double,
pub root_y: c_double,
win_x: c_double,
win_y: c_double,
pub win_x: c_double,
pub win_y: c_double,
buttons: ffi::XIButtonState,
modifiers: ffi::XIModifierState,
group: ffi::XIGroupState,
relative_to_window: bool,
pub group: ffi::XIGroupState,
pub relative_to_window: bool,
}
impl<'a> PointerState<'a> {
@ -93,29 +94,46 @@ impl XConnection {
device_id: c_int,
) -> Result<PointerState<'_>, XError> {
unsafe {
let mut pointer_state: PointerState<'_> = mem::uninitialized();
pointer_state.xconn = self;
pointer_state.relative_to_window = (self.xinput2.XIQueryPointer)(
let mut root = 0;
let mut child = 0;
let mut root_x = 0.0;
let mut root_y = 0.0;
let mut win_x = 0.0;
let mut win_y = 0.0;
let mut buttons = Default::default();
let mut modifiers = Default::default();
let mut group = Default::default();
let relative_to_window = (self.xinput2.XIQueryPointer)(
self.display,
device_id,
window,
&mut pointer_state.root,
&mut pointer_state.child,
&mut pointer_state.root_x,
&mut pointer_state.root_y,
&mut pointer_state.win_x,
&mut pointer_state.win_y,
&mut pointer_state.buttons,
&mut pointer_state.modifiers,
&mut pointer_state.group,
&mut root,
&mut child,
&mut root_x,
&mut root_y,
&mut win_x,
&mut win_y,
&mut buttons,
&mut modifiers,
&mut group,
) == ffi::True;
if let Err(err) = self.check_errors() {
// Running the destrutor would be bad news for us...
mem::forget(pointer_state);
Err(err)
} else {
Ok(pointer_state)
}
self.check_errors()?;
Ok(PointerState {
xconn: self,
root,
child,
root_x,
root_y,
win_x,
win_y,
buttons,
modifiers,
group,
relative_to_window,
})
}
}
@ -123,7 +141,8 @@ impl XConnection {
&self,
ic: ffi::XIC,
key_event: &mut ffi::XKeyEvent,
buffer: &mut [u8],
buffer: *mut u8,
size: usize,
) -> (ffi::KeySym, ffi::Status, c_int) {
let mut keysym: ffi::KeySym = 0;
let mut status: ffi::Status = 0;
@ -131,8 +150,8 @@ impl XConnection {
(self.xlib.Xutf8LookupString)(
ic,
key_event,
buffer.as_mut_ptr() as *mut c_char,
buffer.len() as c_int,
buffer as *mut c_char,
size as c_int,
&mut keysym,
&mut status,
)
@ -141,21 +160,28 @@ impl XConnection {
}
pub fn lookup_utf8(&self, ic: ffi::XIC, key_event: &mut ffi::XKeyEvent) -> String {
let mut buffer: [u8; TEXT_BUFFER_SIZE] = unsafe { mem::uninitialized() };
let (_, status, count) = self.lookup_utf8_inner(ic, key_event, &mut buffer);
// The buffer overflowed, so we'll make a new one on the heap.
if status == ffi::XBufferOverflow {
let mut buffer = Vec::with_capacity(count as usize);
unsafe { buffer.set_len(count as usize) };
let (_, _, new_count) = self.lookup_utf8_inner(ic, key_event, &mut buffer);
// `assume_init` is safe here because the array consists of `MaybeUninit` values,
// which do not require initialization.
let mut buffer: [MaybeUninit<u8>; TEXT_BUFFER_SIZE] =
unsafe { MaybeUninit::uninit().assume_init() };
// If the buffer overflows, we'll make a new one on the heap.
let mut vec;
let (_, status, count) =
self.lookup_utf8_inner(ic, key_event, buffer.as_mut_ptr() as *mut u8, buffer.len());
let bytes = if status == ffi::XBufferOverflow {
vec = Vec::with_capacity(count as usize);
let (_, _, new_count) =
self.lookup_utf8_inner(ic, key_event, vec.as_mut_ptr(), vec.capacity());
debug_assert_eq!(count, new_count);
str::from_utf8(&buffer[..count as usize])
.unwrap_or("")
.to_string()
unsafe { vec.set_len(count as usize) };
&vec[..count as usize]
} else {
str::from_utf8(&buffer[..count as usize])
.unwrap_or("")
.to_string()
}
unsafe { slice::from_raw_parts(buffer.as_ptr() as *const u8, count as usize) }
};
str::from_utf8(bytes).unwrap_or("").to_string()
}
}

View file

@ -18,7 +18,12 @@ pub use self::{
randr::*, window_property::*, wm::*,
};
use std::{mem, ops::BitAnd, os::raw::*, ptr};
use std::{
mem::{self, MaybeUninit},
ops::BitAnd,
os::raw::*,
ptr,
};
use super::{ffi, XConnection, XError};

View file

@ -43,13 +43,14 @@ impl XConnection {
let mut offset = 0;
let mut done = false;
let mut actual_type = 0;
let mut actual_format = 0;
let mut quantity_returned = 0;
let mut bytes_after = 0;
let mut buf: *mut c_uchar = ptr::null_mut();
while !done {
unsafe {
let mut actual_type: ffi::Atom = mem::uninitialized();
let mut actual_format: c_int = mem::uninitialized();
let mut quantity_returned: c_ulong = mem::uninitialized();
let mut bytes_after: c_ulong = mem::uninitialized();
let mut buf: *mut c_uchar = ptr::null_mut();
(self.xlib.XGetWindowProperty)(
self.display,
window,

View file

@ -1,4 +1,14 @@
use std::{cmp, collections::HashSet, env, ffi::CString, mem, os::raw::*, path::Path, sync::Arc};
use std::{
cmp,
collections::HashSet,
env,
ffi::CString,
mem::{self, MaybeUninit},
os::raw::*,
path::Path,
ptr, slice,
sync::Arc,
};
use libc;
use parking_lot::Mutex;
@ -410,11 +420,11 @@ impl UnownedWindow {
unsafe {
// XSetInputFocus generates an error if the window is not visible, so we wait
// until we receive VisibilityNotify.
let mut event = mem::uninitialized();
let mut event = MaybeUninit::uninit();
(xconn.xlib.XIfEvent)(
// This will flush the request buffer IF it blocks.
xconn.display,
&mut event as *mut ffi::XEvent,
event.as_mut_ptr(),
Some(visibility_predicate),
window.xwindow as _,
);
@ -449,19 +459,22 @@ impl UnownedWindow {
let pid_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_PID\0") };
let client_machine_atom = unsafe { self.xconn.get_atom_unchecked(b"WM_CLIENT_MACHINE\0") };
unsafe {
let (hostname, hostname_length) = {
// 64 would suffice for Linux, but 256 will be enough everywhere (as per SUSv2). For instance, this is
// the limit defined by OpenBSD.
const MAXHOSTNAMELEN: usize = 256;
let mut hostname: [c_char; MAXHOSTNAMELEN] = mem::uninitialized();
let status = libc::gethostname(hostname.as_mut_ptr(), hostname.len());
// `assume_init` is safe here because the array consists of `MaybeUninit` values,
// which do not require initialization.
let mut buffer: [MaybeUninit<c_char>; MAXHOSTNAMELEN] =
MaybeUninit::uninit().assume_init();
let status = libc::gethostname(buffer.as_mut_ptr() as *mut c_char, buffer.len());
if status != 0 {
return None;
}
hostname[MAXHOSTNAMELEN - 1] = '\0' as c_char; // a little extra safety
let hostname_length = libc::strlen(hostname.as_ptr());
(hostname, hostname_length as usize)
};
ptr::write(buffer[MAXHOSTNAMELEN - 1].as_mut_ptr() as *mut u8, b'\0'); // a little extra safety
let hostname_length = libc::strlen(buffer.as_ptr() as *const c_char);
let hostname = slice::from_raw_parts(buffer.as_ptr() as *const c_char, hostname_length);
self.xconn
.change_property(
self.xwindow,
@ -1134,13 +1147,13 @@ impl UnownedWindow {
let cursor = unsafe {
// We don't care about this color, since it only fills bytes
// in the pixmap which are not 0 in the mask.
let dummy_color: ffi::XColor = mem::uninitialized();
let mut dummy_color = MaybeUninit::uninit();
let cursor = (self.xconn.xlib.XCreatePixmapCursor)(
self.xconn.display,
pixmap,
pixmap,
&dummy_color as *const _ as *mut _,
&dummy_color as *const _ as *mut _,
dummy_color.as_mut_ptr(),
dummy_color.as_mut_ptr(),
0,
0,
);