Switch to 2024 edition

Cargo.toml

@@ -1,12 +1,12 @@
 [workspace]
 members = [
-  "crates/client",
-  "crates/logging",
-  "crates/runc",
-  "crates/runc-shim",
-  "crates/shim",
-  "crates/shim-protos",
-  "crates/snapshots",
+    "crates/client",
+    "crates/logging",
+    "crates/runc",
+    "crates/runc-shim",
+    "crates/shim",
+    "crates/shim-protos",
+    "crates/snapshots",
 ]
 resolver = "2"
 
@@ -21,7 +21,7 @@ panic = 'abort'
 license = "Apache-2.0"
 repository = "https://github.com/containerd/rust-extensions"
 homepage = "https://containerd.io"
-edition = "2021"
+edition = "2024"
 
 # Common dependencies for all crates
 [workspace.dependencies]

crates/runc-shim/src/console.rs

@@ -57,18 +57,18 @@ impl ConsoleSocket {
 
     // async drop is not supported yet, we can only call clean manually after socket received
     pub async fn clean(self) {
-        if self.rmdir {
-            if let Some(tmp_socket_dir) = self.path.parent() {
-                tokio::fs::remove_dir_all(tmp_socket_dir)
-                    .await
-                    .unwrap_or_else(|e| {
-                        warn!(
-                            "remove tmp console socket path {} : {}",
-                            tmp_socket_dir.display(),
-                            e
-                        )
-                    })
-            }
+        if self.rmdir
+            && let Some(tmp_socket_dir) = self.path.parent()
+        {
+            tokio::fs::remove_dir_all(tmp_socket_dir)
+                .await
+                .unwrap_or_else(|e| {
+                    warn!(
+                        "remove tmp console socket path {} : {}",
+                        tmp_socket_dir.display(),
+                        e
+                    )
+                })
         }
     }
 }

crates/runc-shim/src/runc.rs

@@ -307,12 +307,9 @@ impl ProcessLifecycle<InitProcess> for RuncInitLifecycle {
                 Some(&runc::options::DeleteOpts { force: true }),
             )
             .await
+            && !e.to_string().to_lowercase().contains("does not exist")
         {
-            if !e.to_string().to_lowercase().contains("does not exist") {
-                return Err(
-                    runtime_error(&p.lifecycle.bundle, e, "OCI runtime delete failed").await,
-                );
-            }
+            return Err(runtime_error(&p.lifecycle.bundle, e, "OCI runtime delete failed").await);
         }
         umount_recursive(Path::new(&self.bundle).join("rootfs").to_str(), 0)?;
         self.exit_signal.signal();
@@ -698,36 +695,36 @@ pub async fn copy_io(pio: &ProcessIO, stdio: &Stdio, exit_signal: Arc<ExitSignal
             }
         }
 
-        if let Some(r) = io.stderr() {
-            if !stdio.stderr.is_empty() {
-                debug!("copy_io: pipe stderr from to {}", stdio.stderr.as_str());
-                let stderr = handle_file_open(|| async {
-                    OpenOptions::new()
-                        .write(true)
-                        .open(stdio.stderr.as_str())
-                        .await
-                })
-                .await
-                .map_err(io_error!(e, "open stderr"))?;
-                // open a read to make sure even if the read end of containerd shutdown,
-                // copy still continue until the restart of containerd succeed
-                let stderr_r = handle_file_open(|| async {
-                    OpenOptions::new()
-                        .read(true)
-                        .open(stdio.stderr.as_str())
-                        .await
-                })
-                .await
-                .map_err(io_error!(e, "open stderr for read"))?;
-                spawn_copy(
-                    r,
-                    stderr,
-                    exit_signal,
-                    Some(move || {
-                        drop(stderr_r);
-                    }),
-                );
-            }
+        if let Some(r) = io.stderr()
+            && !stdio.stderr.is_empty()
+        {
+            debug!("copy_io: pipe stderr from to {}", stdio.stderr.as_str());
+            let stderr = handle_file_open(|| async {
+                OpenOptions::new()
+                    .write(true)
+                    .open(stdio.stderr.as_str())
+                    .await
+            })
+            .await
+            .map_err(io_error!(e, "open stderr"))?;
+            // open a read to make sure even if the read end of containerd shutdown,
+            // copy still continue until the restart of containerd succeed
+            let stderr_r = handle_file_open(|| async {
+                OpenOptions::new()
+                    .read(true)
+                    .open(stdio.stderr.as_str())
+                    .await
+            })
+            .await
+            .map_err(io_error!(e, "open stderr for read"))?;
+            spawn_copy(
+                r,
+                stderr,
+                exit_signal,
+                Some(move || {
+                    drop(stderr_r);
+                }),
+            );
         }
     }
 
@@ -834,11 +831,10 @@ async fn wait_pid(pid: i32, s: Subscription) -> i32 {
             subject: Subject::Pid(epid),
             exit_code: code,
         }) = s.rx.recv().await
+            && pid == epid
         {
-            if pid == epid {
-                monitor_unsubscribe(s.id).await.unwrap_or_default();
-                return code;
-            }
+            monitor_unsubscribe(s.id).await.unwrap_or_default();
+            return code;
         }
     }
 }

crates/runc/src/asynchronous/runc.rs

@@ -55,18 +55,18 @@ impl Runc {
         unsafe {
             cmd.pre_exec(move || {
                 #[cfg(target_os = "linux")]
-                if let Ok(thp) = std::env::var("THP_DISABLED") {
-                    if let Ok(thp_disabled) = thp.parse::<bool>() {
-                        let ret = libc::prctl(
-                            libc::PR_SET_THP_DISABLE,
-                            if thp_disabled { 1u64 } else { 0u64 },
-                            0,
-                            0,
-                            0,
-                        );
-                        if ret < 0 {
-                            debug!("set_thp_disable err: {}", std::io::Error::last_os_error());
-                        }
+                if let Ok(thp) = std::env::var("THP_DISABLED")
+                    && let Ok(thp_disabled) = thp.parse::<bool>()
+                {
+                    let ret = libc::prctl(
+                        libc::PR_SET_THP_DISABLE,
+                        if thp_disabled { 1u64 } else { 0u64 },
+                        0,
+                        0,
+                        0,
+                    );
+                    if ret < 0 {
+                        debug!("set_thp_disable err: {}", std::io::Error::last_os_error());
                     }
                 }
                 Ok(())

crates/shim/src/asynchronous/mod.rs

@@ -515,14 +515,14 @@ async fn remove_socket_silently(address: &str) {
 #[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))]
 async fn remove_socket(address: &str) -> Result<()> {
     let path = parse_sockaddr(address);
-    if let Ok(md) = Path::new(path).metadata() {
-        if md.file_type().is_socket() {
-            tokio::fs::remove_file(path).await.map_err(io_error!(
-                e,
-                "failed to remove socket {}",
-                address
-            ))?;
-        }
+    if let Ok(md) = Path::new(path).metadata()
+        && md.file_type().is_socket()
+    {
+        tokio::fs::remove_file(path).await.map_err(io_error!(
+            e,
+            "failed to remove socket {}",
+            address
+        ))?;
     }
     Ok(())
 }

crates/shim/src/logger.rs

@@ -221,6 +221,11 @@ mod tests {
     use super::*;
     use crate::Config;
 
+    // Only this test touches `LOG_ENV`, and it does so from a single thread.
+    fn set_log_env(value: &str) {
+        unsafe { std::env::set_var(LOG_ENV, value) };
+    }
+
     #[test]
     fn test_init_log_level() -> Result<(), Error> {
         let config = Config::default();
@@ -233,38 +238,38 @@ mod tests {
         assert_eq!(log::LevelFilter::Debug, log::max_level());
 
         // ENV different than default
-        std::env::set_var(LOG_ENV, "error");
+        set_log_env("error");
         configure_logging_level(false, &config.default_log_level);
         assert_eq!(log::LevelFilter::Error, log::max_level());
 
-        std::env::set_var(LOG_ENV, "warn");
+        set_log_env("warn");
         configure_logging_level(false, &config.default_log_level);
         assert_eq!(log::LevelFilter::Warn, log::max_level());
 
-        std::env::set_var(LOG_ENV, "off");
+        set_log_env("off");
         configure_logging_level(false, &config.default_log_level);
         assert_eq!(log::LevelFilter::Off, log::max_level());
 
-        std::env::set_var(LOG_ENV, "trace");
+        set_log_env("trace");
         configure_logging_level(false, &config.default_log_level);
         assert_eq!(log::LevelFilter::Trace, log::max_level());
 
-        std::env::set_var(LOG_ENV, "debug");
+        set_log_env("debug");
         configure_logging_level(false, &config.default_log_level);
 
         // ENV Different than default from debug flag
         configure_logging_level(true, &config.default_log_level);
         assert_eq!(log::LevelFilter::Debug, log::max_level());
 
-        std::env::set_var(LOG_ENV, "trace");
+        set_log_env("trace");
         configure_logging_level(true, &config.default_log_level);
         assert_eq!(log::LevelFilter::Trace, log::max_level());
 
-        std::env::set_var(LOG_ENV, "info");
+        set_log_env("info");
         configure_logging_level(true, &config.default_log_level);
         assert_eq!(log::LevelFilter::Debug, log::max_level());
 
-        std::env::set_var(LOG_ENV, "off");
+        set_log_env("off");
         configure_logging_level(true, &config.default_log_level);
         assert_eq!(log::LevelFilter::Debug, log::max_level());
         Ok(())

crates/shim/src/mount_linux.rs

@@ -896,10 +896,10 @@ pub fn umount_recursive(target: Option<&str>, flags: i32) -> Result<()> {
 
 fn umount_all(target: Option<String>, flags: i32) -> Result<()> {
     if let Some(target) = target {
-        if let Err(e) = std::fs::metadata(target.clone()) {
-            if e.kind() == std::io::ErrorKind::NotFound {
-                return Ok(());
-            }
+        if let Err(e) = std::fs::metadata(target.clone())
+            && e.kind() == std::io::ErrorKind::NotFound
+        {
+            return Ok(());
         }
         loop {
             if let Err(e) = nix::mount::umount2(
@@ -997,11 +997,11 @@ where
             }
             mount_info.major = str::parse::<u32>(major_minor[0]).ok()?;
             mount_info.minor = str::parse::<u32>(major_minor[1]).ok()?;
-            if let Some(f) = &f {
-                if f(mount_info.clone()) {
-                    // skip this mountpoint. This mountpoint is not the container's mountpoint
-                    return None;
-                }
+            if let Some(f) = &f
+                && f(mount_info.clone())
+            {
+                // skip this mountpoint. This mountpoint is not the container's mountpoint
+                return None;
             }
             Some(mount_info)
         })
@@ -1139,11 +1139,11 @@ mod tests {
         ];
         // mount target.
         let result = mount_rootfs(Some("overlay"), Some("overlay"), &options, &target);
-        if let Err(err) = &result {
-            if crate::error::is_permission_error(err) {
-                eprintln!("skipping test_mount_rootfs_umount_recursive: {err}");
-                return;
-            }
+        if let Err(err) = &result
+            && crate::error::is_permission_error(err)
+        {
+            eprintln!("skipping test_mount_rootfs_umount_recursive: {err}");
+            return;
         }
         assert!(result.is_ok(), "{result:?}");
         let mut mountinfo = get_mounts(Some(prefix_filter(
@@ -1182,11 +1182,11 @@ mod tests {
             direct: true,
         };
         let result = setup_loop(backing_file, params);
-        if let Err(err) = &result {
-            if crate::error::is_permission_error(err) {
-                eprintln!("skipping test_setup_loop_dev: {err}");
-                return;
-            }
+        if let Err(err) = &result
+            && crate::error::is_permission_error(err)
+        {
+            eprintln!("skipping test_setup_loop_dev: {err}");
+            return;
         }
         assert!(result.is_ok(), "{result:?}");
     }

crates/shim/src/synchronous/mod.rs

@@ -371,7 +371,7 @@ fn setup_signals(_config: &Config) -> Option<AppSignals> {
 
 #[cfg(windows)]
 unsafe extern "system" fn signal_handler(_: u32) -> i32 {
-    ReleaseSemaphore(SEMAPHORE, 1, ptr::null_mut());
+    unsafe { ReleaseSemaphore(SEMAPHORE, 1, ptr::null_mut()) };
     1
 }
 
@@ -462,10 +462,10 @@ fn remove_socket(address: &str) -> Result<()> {
     #[cfg(unix)]
     {
         let path = parse_sockaddr(address);
-        if let Ok(md) = Path::new(path).metadata() {
-            if md.file_type().is_socket() {
-                fs::remove_file(path).map_err(io_error!(e, "remove socket"))?;
-            }
+        if let Ok(md) = Path::new(path).metadata()
+            && md.file_type().is_socket()
+        {
+            fs::remove_file(path).map_err(io_error!(e, "remove socket"))?;
         }
     }
 

crates/shim/src/synchronous/monitor.rs

@@ -141,10 +141,9 @@ pub fn wait_pid(pid: i32, s: Subscription) -> i32 {
             subject: Subject::Pid(epid),
             exit_code: code,
         }) = s.rx.recv()
+            && pid == epid
         {
-            if pid == epid {
-                return code;
-            }
+            return code;
         }
     }
 }

rust-toolchain.toml

@@ -1,3 +1,3 @@
 [toolchain]
 channel = "1.91"
-components = ["rustfmt", "clippy", "llvm-tools"]
+components = ["rustfmt", "clippy", "llvm-tools", "rust-analyzer"]

rustfmt.toml

@@ -1,4 +1,5 @@
 newline_style = "Unix"
+style_edition = "2021" # Keep 2021 formatting (mainly import sort order) across the 2024 edition bump
 unstable_features = true # Cargo fmt now needs to be called with `cargo +nightly fmt`
 group_imports = "StdExternalCrate" # Create 3 groups: std, external crates, and self.
 imports_granularity = "Crate" # Merge imports from the same crate into a single use statement