Review the unsafe fn (1st round).

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)

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()
 }
 

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<Option<(Library, PathBuf)>> {
         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!(

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<Vec<u8>> {
     use winapi::um::iptypes::IP_ADAPTER_ADDRESSES_LH;
 
@@ -43,13 +46,16 @@ unsafe fn get_adapters_addresses(af_spec: i32) -> ZResult<Vec<u8>> {
     let mut buffer: Vec<u8>;
     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;
         }

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::<T>()` bytes from `value`.
 unsafe fn setsockopt<T>(
     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::<T>() as libc::socklen_t,
-    ) == 0
+    // SAFETY: Call the underlying C function `setsockopt`.
+    if unsafe {
+        libc::setsockopt(
+            socket,
+            level,
+            name,
+            value,
+            mem::size_of::<T>() as libc::socklen_t,
+        )
+    } == 0
     {
         Ok(())
     } else {

io/zenoh-links/zenoh-link-udp/src/pktinfo/pktinfo_windows.rs

@@ -82,12 +82,21 @@ fn locate_wsarecvmsg(socket: RawSocket) -> io::Result<WSARecvMsgExtension> {
     }
 }
 
+/// # 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::<T>()` bytes from `payload`.
 unsafe fn setsockopt<T>(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::<T>() as i32) == 0 {
+    // SAFETY: Call the underlying C function `setsockopt`.
+    if unsafe { WinSock::setsockopt(socket as _, opt, val, payload, mem::size_of::<T>() as i32) }
+        == 0
+    {
         Ok(())
     } else {
         Err(Error::from_raw_os_error(WinSock::WSAGetLastError()))

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,