ci: Fix clippy warnings (#23)

Summary:
Fixes all current clippy warnings[1] so CI is green again.

Fixing the warning `clippy::needless_pass_by_ref_mut` became a little involved. The internal version of clippy isn't recent enough to have this warning and so just doing `#[allow(clippy::needless_pass_by_ref_mut)]` leads to another error. The resulting change fixes the clippy warning and gets rid of some of the shenanigans I was doing to avoid allocating path buffers within the child process.

[1]: https://github.com/facebookexperimental/reverie/actions/runs/6164196219/job/16729396694

Pull Request resolved: #23

Reviewed By: VladimirMakaev

Differential Revision: D49209202

Pulled By: jasonwhite

fbshipit-source-id: b03ff432783910bef11fc239d146659dc2c0db30

reverie-examples/chunky_print.rs

@@ -74,7 +74,7 @@ impl GlobalTool for ChunkyPrintGlobal {
         let mut mg = self.0.lock().unwrap();
         match m {
             Msg::Print(w, s) => {
-                let v = mg.printbuf.entry(from).or_insert_with(Vec::new);
+                let v = mg.printbuf.entry(from).or_default();
                 v.push((w, s));
             }
             Msg::Tick => {

reverie-process/src/fd.rs

@@ -10,7 +10,6 @@ use core::pin::Pin;
 use core::task::Context;
 use core::task::Poll;
 use std::ffi::CStr;
-use std::ffi::CString;
 use std::io;
 use std::io::Read;
 use std::io::Write;
@@ -378,20 +377,37 @@ pub fn is_dir(path: *const libc::c_char) -> bool {
     }
 }
 
-fn cstring_as_slice(s: &mut CString) -> &mut [libc::c_char] {
-    let bytes = s.as_bytes_with_nul();
-    unsafe {
-        // This is safe because we are already provided a mutable `CString` and
-        // we don't alias the two mutable references.
-        core::slice::from_raw_parts_mut(bytes.as_ptr() as *mut libc::c_char, bytes.len())
+/// Copies the bytes of a `CStr` to a buffer. Helpful to avoid allocations when
+/// performing path operations in a child process that hasn't called `execve`
+/// yet.
+fn copy_cstr_to_slice<'a>(
+    s: &CStr,
+    buf: &'a mut [libc::c_char],
+) -> Result<&'a mut [libc::c_char], Errno> {
+    let bytes = s.to_bytes_with_nul();
+
+    if bytes.len() > buf.len() {
+        return Err(Errno::ENAMETOOLONG);
     }
+
+    unsafe {
+        core::ptr::copy_nonoverlapping(
+            bytes.as_ptr() as *const libc::c_char,
+            buf.as_mut_ptr(),
+            bytes.len(),
+        )
+    };
+
+    Ok(&mut buf[0..bytes.len()])
 }
 
 /// Creates every path component in `path` without allocating. This is done by
-/// replacing each `/` with a NUL terminator as needed (and then changing the
-/// `\0` back to `/` afterwards).
-pub fn create_dir_all(path: &mut CString, mode: libc::mode_t) -> Result<(), Errno> {
-    create_dir_all_(cstring_as_slice(path), mode)
+/// copying the path to a static buffer and replacing each `/` with a NUL
+/// terminator as needed (and then changing the `\0` back to `/` afterwards).
+pub fn create_dir_all(path: &CStr, mode: libc::mode_t) -> Result<(), Errno> {
+    let mut buf = ['\0' as libc::c_char; libc::PATH_MAX as usize];
+    let path = copy_cstr_to_slice(path, &mut buf)?;
+    create_dir_all_(path, mode)
 }
 
 /// Helper function. The last character in the path is always `\0`.
@@ -431,11 +447,13 @@ fn create_dir_all_(path: &mut [libc::c_char], mode: libc::mode_t) -> Result<(),
 
 /// Creates an empty file at `path` without allocating.
 pub fn touch_path(
-    path: &mut CString,
+    path: &CStr,
     file_mode: libc::mode_t,
     dir_mode: libc::mode_t,
 ) -> Result<(), Errno> {
-    touch_path_(cstring_as_slice(path), file_mode, dir_mode)
+    let mut buf = ['\0' as libc::c_char; libc::PATH_MAX as usize];
+    let path = copy_cstr_to_slice(path, &mut buf)?;
+    touch_path_(path, file_mode, dir_mode)
 }
 
 /// Helper function. The last character in the path is always `\0`.
@@ -493,12 +511,30 @@ where
 
 #[cfg(test)]
 mod tests {
+    use std::ffi::CString;
     use std::os::unix::ffi::OsStrExt;
 
     use const_cstr::const_cstr;
 
     use super::*;
 
+    #[test]
+    fn test_copy_cstr_to_slice() {
+        const BYTES: &[u8] = b"/foo/bar\0";
+        let s = CStr::from_bytes_with_nul(BYTES).unwrap();
+
+        // Buffer exactly the right size.
+        let mut buf = [0; BYTES.len()];
+        assert_eq!(
+            copy_cstr_to_slice(s, &mut buf).unwrap().len(),
+            s.to_bytes_with_nul().len()
+        );
+
+        // Buffer too small to hold last NUL byte.
+        let mut buf = [0; BYTES.len() - 1];
+        assert_eq!(copy_cstr_to_slice(s, &mut buf), Err(Errno::ENAMETOOLONG));
+    }
+
     #[test]
     fn test_is_dir() {
         assert!(is_dir(const_cstr!("/").as_ptr()));
@@ -524,39 +560,33 @@ mod tests {
     #[test]
     fn test_create_dir_all() {
         let tempdir = tempfile::TempDir::new().unwrap();
-        let mut path = CString::new(
+        let path = CString::new(
             tempdir
                 .path()
                 .join("some/path/to/a/dir")
                 .into_os_string()
                 .as_bytes(),
         )
         .unwrap();
-        let path2 = path.clone();
-
-        create_dir_all(&mut path, 0o777).unwrap();
 
-        assert_eq!(path, path2);
+        create_dir_all(&path, 0o777).unwrap();
 
         assert!(is_dir(path.as_ptr()));
     }
 
     #[test]
     fn test_touch_path() {
         let tempdir = tempfile::TempDir::new().unwrap();
-        let mut path = CString::new(
+        let path = CString::new(
             tempdir
                 .path()
                 .join("some/path/to/a/file")
                 .into_os_string()
                 .as_bytes(),
         )
         .unwrap();
-        let path2 = path.clone();
-
-        touch_path(&mut path, 0o666, 0o777).unwrap();
 
-        assert_eq!(path, path2);
+        touch_path(&path, 0o666, 0o777).unwrap();
 
         assert!(FileType::new(path.as_ptr()).unwrap().is_file());
     }

reverie-process/src/lib.rs

@@ -227,9 +227,7 @@ mod tests {
             .trim_end()
             .split('\n')
             .map(|line| {
-                let mut items = line.splitn(2, ':');
-                let first = items.next().unwrap();
-                let second = items.next().unwrap();
+                let (first, second) = line.split_once(':').unwrap();
                 (first, second.trim())
             })
             .collect()
@@ -655,6 +653,7 @@ mod tests {
             .arg("/proc/self/status")
             .seccomp(filter)
             .seccomp_notify()
+            .stdout(Stdio::null())
             .spawn()
             .unwrap();
 

reverie-process/src/mount.rs

@@ -292,9 +292,9 @@ impl Mount {
             // a different tmpfs.
             if let Some(src) = &self.source {
                 if FileType::new(src.as_ptr())?.is_dir() {
-                    create_dir_all(&mut self.target, 0o777)?;
+                    create_dir_all(&self.target, 0o777)?;
                 } else {
-                    touch_path(&mut self.target, 0o666, 0o777)?;
+                    touch_path(&self.target, 0o666, 0o777)?;
                 }
             }
         }

reverie-process/src/seccomp/notif.rs

@@ -283,7 +283,7 @@ impl futures::stream::Stream for SeccompNotif {
 /// currently pending, the operation blocks until an event occurs.
 ///
 /// NOTE: This is only available since Linux 5.0.
-fn seccomp_notif_recv(fd: &mut Fd) -> io::Result<seccomp_notif> {
+fn seccomp_notif_recv(fd: &Fd) -> io::Result<seccomp_notif> {
     // According to the docs, this struct must be zeroed out first.
     let mut response = core::mem::MaybeUninit::<seccomp_notif>::zeroed();