diff --git a/commons/zenoh-stats/src/keys.rs b/commons/zenoh-stats/src/keys.rs index 4ab22db143..aa8acfb711 100644 --- a/commons/zenoh-stats/src/keys.rs +++ b/commons/zenoh-stats/src/keys.rs @@ -56,6 +56,7 @@ impl StatsKeysTree { if cache.generation.load(Ordering::Acquire) != self.generation { self.update_cache(cache, keyexpr); } + // SAFETY: Dereference the raw pointer. return unsafe { &*cache.keys.get() }.clone(); } self.compute_keys(keyexpr) diff --git a/commons/zenoh-util/src/ffi/mod.rs b/commons/zenoh-util/src/ffi/mod.rs index c1c2e24815..265a7cfa1f 100644 --- a/commons/zenoh-util/src/ffi/mod.rs +++ b/commons/zenoh-util/src/ffi/mod.rs @@ -15,24 +15,36 @@ pub mod win; /// # Safety -/// Dereferences raw pointer argument +/// Dereferences raw pointer argument. +/// ptr should be a valid string pointer. pub unsafe fn pwstr_to_string(ptr: *mut u16) -> String { use std::slice::from_raw_parts; - let len = (0_usize..) - .find(|&n| *ptr.add(n) == 0) - .expect("Couldn't find null terminator"); - let array: &[u16] = from_raw_parts(ptr, len); + + // SAFETY: Dereference the raw pointer and call from_raw_parts. + let array: &[u16] = unsafe { + let len = (0_usize..) + .find(|&n| *ptr.add(n) == 0) + .expect("Couldn't find null terminator"); + from_raw_parts(ptr, len) + }; + String::from_utf16_lossy(array) } /// # Safety -/// Dereferences raw pointer argument +/// Dereferences raw pointer argument. +/// ptr should be a valid string pointer. pub unsafe fn pstr_to_string(ptr: *mut i8) -> String { use std::slice::from_raw_parts; - let len = (0_usize..) - .find(|&n| *ptr.add(n) == 0) - .expect("Couldn't find null terminator"); - let array: &[u8] = from_raw_parts(ptr as *const u8, len); + + // SAFETY: Dereference the raw pointer and call from_raw_parts. + let array: &[u8] = unsafe { + let len = (0_usize..) + .find(|&n| *ptr.add(n) == 0) + .expect("Couldn't find null terminator"); + from_raw_parts(ptr as *const u8, len) + }; + String::from_utf8_lossy(array).to_string() } diff --git a/commons/zenoh-util/src/lib_loader.rs b/commons/zenoh-util/src/lib_loader.rs index 082bb04839..732ec920a8 100644 --- a/commons/zenoh-util/src/lib_loader.rs +++ b/commons/zenoh-util/src/lib_loader.rs @@ -73,6 +73,7 @@ impl LibLoader { /// /// This function calls [libloading::Library::new()](https://docs.rs/libloading/0.7.0/libloading/struct.Library.html#method.new) /// which is unsafe. + /// The library should be valid, or it might cause the undefined behavior. pub unsafe fn load_file(path: &str) -> ZResult<(Library, PathBuf)> { let path = Self::str_to_canonical_path(path)?; @@ -81,7 +82,8 @@ impl LibLoader { } else if !path.is_file() { bail!("Library file '{}' is not a file", path.display()) } else { - Ok((Library::new(path.clone())?, path)) + // SAFETY: Call unsafe `libloading::Library::new()`. + unsafe { Ok((Library::new(path.clone())?, path)) } } } @@ -94,6 +96,7 @@ impl LibLoader { /// /// This function calls [libloading::Library::new()](https://docs.rs/libloading/0.7.0/libloading/struct.Library.html#method.new) /// which is unsafe. + /// The library should be valid, or it might cause the undefined behavior. pub unsafe fn search_and_load(&self, name: &str) -> ZResult> { let filename = format!("{}{}{}", *LIB_PREFIX, name, *LIB_SUFFIX); let filename_ostr = OsString::from(&filename); @@ -111,7 +114,8 @@ impl LibLoader { for entry in read_dir.flatten() { if entry.file_name() == filename_ostr { let path = entry.path(); - return Ok(Some((Library::new(path.clone())?, path))); + // SAFETY: Call unsafe `libloading::Library::new()`. + return unsafe { Ok(Some((Library::new(path.clone())?, path))) }; } } } @@ -134,6 +138,7 @@ impl LibLoader { /// /// This function calls [libloading::Library::new()](https://docs.rs/libloading/0.7.0/libloading/struct.Library.html#method.new) /// which is unsafe. + /// The library should be valid, or it might cause the undefined behavior. pub unsafe fn load_all_with_prefix( &self, prefix: Option<&str>, @@ -157,9 +162,12 @@ impl LibLoader { [(lib_prefix.len())..(filename.len() - LIB_SUFFIX.len())]; let path = entry.path(); if !result.iter().any(|(_, _, n)| n == name) { - match Library::new(path.as_os_str()) { - Ok(lib) => result.push((lib, path, name.to_string())), - Err(err) => warn!("{}", err), + // SAFETY: Call unsafe `libloading::Library::new()`. + unsafe { + match Library::new(path.as_os_str()) { + Ok(lib) => result.push((lib, path, name.to_string())), + Err(err) => warn!("{}", err), + } } } else { debug!( diff --git a/commons/zenoh-util/src/net/mod.rs b/commons/zenoh-util/src/net/mod.rs index e8d3760f99..49af304e59 100644 --- a/commons/zenoh-util/src/net/mod.rs +++ b/commons/zenoh-util/src/net/mod.rs @@ -34,6 +34,9 @@ lazy_static! { } #[cfg(windows)] +/// # Safety +/// The caller must ensure the `af_spec`` is valid, which will be used by +/// `winapi::um::iphlpapi::GetAdaptersAddresses`. unsafe fn get_adapters_addresses(af_spec: i32) -> ZResult> { use winapi::um::iptypes::IP_ADAPTER_ADDRESSES_LH; @@ -43,13 +46,16 @@ unsafe fn get_adapters_addresses(af_spec: i32) -> ZResult> { let mut buffer: Vec; loop { buffer = Vec::with_capacity(size as usize); - ret = winapi::um::iphlpapi::GetAdaptersAddresses( - af_spec.try_into().unwrap(), - 0, - std::ptr::null_mut(), - buffer.as_mut_ptr() as *mut IP_ADAPTER_ADDRESSES_LH, - &mut size, - ); + // SAFETY: Call the unsafe function `GetAdaptersAddresses`. + ret = unsafe { + winapi::um::iphlpapi::GetAdaptersAddresses( + af_spec.try_into().unwrap(), + 0, + std::ptr::null_mut(), + buffer.as_mut_ptr() as *mut IP_ADAPTER_ADDRESSES_LH, + &mut size, + ) + }; if ret != winapi::shared::winerror::ERROR_BUFFER_OVERFLOW { break; } diff --git a/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_unix.rs b/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_unix.rs index feb6891db5..742f339f2b 100644 --- a/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_unix.rs +++ b/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_unix.rs @@ -25,6 +25,12 @@ use std::{ use socket2::SockAddr; use tokio::{io::Interest, net::UdpSocket}; +/// # Safety +/// The caller must ensure that: +/// - `socket` is a valid file descriptor for a socket. +/// - The combination of `level`, `name`, and the type `T` of `value` is a valid +/// socket option. The underlying C function `setsockopt` will read +/// `mem::size_of::()` bytes from `value`. unsafe fn setsockopt( socket: libc::c_int, level: libc::c_int, @@ -35,13 +41,16 @@ where T: Copy, { let value = &value as *const T as *const libc::c_void; - if libc::setsockopt( - socket, - level, - name, - value, - mem::size_of::() as libc::socklen_t, - ) == 0 + // SAFETY: Call the underlying C function `setsockopt`. + if unsafe { + libc::setsockopt( + socket, + level, + name, + value, + mem::size_of::() as libc::socklen_t, + ) + } == 0 { Ok(()) } else { diff --git a/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_windows.rs b/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_windows.rs index f3ac2ebc2d..28b30e854e 100644 --- a/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_windows.rs +++ b/io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_windows.rs @@ -82,12 +82,21 @@ fn locate_wsarecvmsg(socket: RawSocket) -> io::Result { } } +/// # Safety +/// The caller must ensure that: +/// - `socket` is a valid file descriptor for a socket. +/// - The combination of `opt`, `val`, and the type `T` of `payload` is a valid +/// socket option. The underlying C function `setsockopt` will read +/// `mem::size_of::()` bytes from `payload`. unsafe fn setsockopt(socket: RawSocket, opt: i32, val: i32, payload: T) -> io::Result<()> where T: Copy, { let payload = &payload as *const T as PCSTR; - if WinSock::setsockopt(socket as _, opt, val, payload, mem::size_of::() as i32) == 0 { + // SAFETY: Call the underlying C function `setsockopt`. + if unsafe { WinSock::setsockopt(socket as _, opt, val, payload, mem::size_of::() as i32) } + == 0 + { Ok(()) } else { Err(Error::from_raw_os_error(WinSock::WSAGetLastError())) diff --git a/zenoh/src/api/queryable.rs b/zenoh/src/api/queryable.rs index 087d594465..2e531e862f 100644 --- a/zenoh/src/api/queryable.rs +++ b/zenoh/src/api/queryable.rs @@ -388,15 +388,15 @@ impl Query { Ok(self.parameters().reply_key_expr_any()) } - /// Constructs an empty Query without payload or attachment, referencing the same inner query. + /// Constructs an empty Query without payload or attachment. /// /// # Examples /// ``` /// # fn main() { - /// let query = unsafe { zenoh::query::Query::empty() }; + /// let query = zenoh::query::Query::empty(); /// # } #[zenoh_macros::internal] - pub unsafe fn empty() -> Self { + pub fn empty() -> Self { Query { inner: Arc::new(QueryInner::empty()), eid: 0,