From 949dde89c7c97c72c09f2aeb19522acabc8f499e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BChlbacher?= Date: Thu, 27 Jun 2024 15:14:55 +0200 Subject: [PATCH] feat: use `ExitCode` over `std::process:exit()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should provide us with better outputs on process failure, also makes unwinding better and is generally recommended over `exit()`. Signed-off-by: Thomas Mühlbacher --- src/bcachefs.rs | 28 +++++++++++++++------------- src/commands/completions.rs | 3 +-- src/commands/list.rs | 11 +++-------- src/commands/mount.rs | 10 ++-------- src/commands/subvolume.rs | 15 ++++++++------- 5 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/bcachefs.rs b/src/bcachefs.rs index 643ac0484..26422abd6 100644 --- a/src/bcachefs.rs +++ b/src/bcachefs.rs @@ -3,7 +3,10 @@ mod key; mod logging; mod wrappers; -use std::ffi::{c_char, CString}; +use std::{ + ffi::{c_char, CString}, + process::{ExitCode, Termination}, +}; use bch_bindgen::c; @@ -71,7 +74,7 @@ fn handle_c_command(mut argv: Vec, symlink_cmd: Option<&str>) -> i32 { } } -fn main() { +fn main() -> ExitCode { let args: Vec = std::env::args().collect(); let symlink_cmd: Option<&str> = if args[0].contains("mkfs") { @@ -89,7 +92,7 @@ fn main() { if symlink_cmd.is_none() && args.len() < 2 { println!("missing command"); unsafe { c::bcachefs_usage() }; - std::process::exit(1); + return ExitCode::from(1); } unsafe { c::raid_init() }; @@ -99,15 +102,14 @@ fn main() { None => args[1].as_str(), }; - let ret = match cmd { - "completions" => commands::completions(args[1..].to_vec()), - "list" => commands::list(args[1..].to_vec()), - "mount" => commands::mount(args, symlink_cmd), - "subvolume" => commands::subvolume(args[1..].to_vec()), - _ => handle_c_command(args, symlink_cmd), - }; - - if ret != 0 { - std::process::exit(1); + match cmd { + "completions" => { + commands::completions(args[1..].to_vec()); + ExitCode::SUCCESS + } + "list" => commands::list(args[1..].to_vec()).report(), + "mount" => commands::mount(args, symlink_cmd).report(), + "subvolume" => commands::subvolume(args[1..].to_vec()).report(), + _ => ExitCode::from(u8::try_from(handle_c_command(args, symlink_cmd)).unwrap()), } } diff --git a/src/commands/completions.rs b/src/commands/completions.rs index d4e985692..e05934ca7 100644 --- a/src/commands/completions.rs +++ b/src/commands/completions.rs @@ -12,8 +12,7 @@ fn print_completions(gen: G, cmd: &mut Command) { generate(gen, cmd, cmd.get_name().to_string(), &mut io::stdout()); } -pub fn completions(argv: Vec) -> i32 { +pub fn completions(argv: Vec) { let cli = Cli::parse_from(argv); print_completions(cli.shell, &mut super::Cli::command()); - 0 } diff --git a/src/commands/list.rs b/src/commands/list.rs index b7a42916f..b1bf144a5 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -1,3 +1,4 @@ +use anyhow::Result; use bch_bindgen::bcachefs; use bch_bindgen::bkey::BkeySC; use bch_bindgen::btree::BtreeIter; @@ -7,7 +8,6 @@ use bch_bindgen::btree::BtreeTrans; use bch_bindgen::fs::Fs; use bch_bindgen::opt_set; use clap::Parser; -use log::error; use std::io::{stdout, IsTerminal}; use crate::logging; @@ -201,16 +201,11 @@ fn cmd_list_inner(opt: &Cli) -> anyhow::Result<()> { } } -pub fn list(argv: Vec) -> i32 { +pub fn list(argv: Vec) -> Result<()> { let opt = Cli::parse_from(argv); // TODO: centralize this on the top level CLI logging::setup(opt.quiet, opt.verbose, opt.colorize); - if let Err(e) = cmd_list_inner(&opt) { - error!("Fatal error: {}", e); - 1 - } else { - 0 - } + cmd_list_inner(&opt) } diff --git a/src/commands/mount.rs b/src/commands/mount.rs index 05c8979d0..7eedf1a88 100644 --- a/src/commands/mount.rs +++ b/src/commands/mount.rs @@ -381,7 +381,7 @@ fn cmd_mount_inner(opt: Cli) -> Result<()> { } } -pub fn mount(mut argv: Vec, symlink_cmd: Option<&str>) -> i32 { +pub fn mount(mut argv: Vec, symlink_cmd: Option<&str>) -> Result<()> { // If the bcachefs tool is being called as "bcachefs mount dev ..." (as opposed to via a // symlink like "/usr/sbin/mount.bcachefs dev ...", then we need to pop the 0th argument // ("bcachefs") since the CLI parser here expects the device at position 1. @@ -394,11 +394,5 @@ pub fn mount(mut argv: Vec, symlink_cmd: Option<&str>) -> i32 { // TODO: centralize this on the top level CLI logging::setup(opt.quiet, opt.verbose, opt.colorize); - if let Err(e) = cmd_mount_inner(opt) { - error!("Fatal error: {}", e); - 1 - } else { - info!("Successfully mounted"); - 0 - } + cmd_mount_inner(opt) } diff --git a/src/commands/subvolume.rs b/src/commands/subvolume.rs index b059ff4a7..bb0141c4c 100644 --- a/src/commands/subvolume.rs +++ b/src/commands/subvolume.rs @@ -1,5 +1,6 @@ use std::{env, path::PathBuf}; +use anyhow::{Context, Result}; use bch_bindgen::c::BCH_SUBVOL_SNAPSHOT_RO; use clap::{Parser, Subcommand}; @@ -36,7 +37,7 @@ enum Subcommands { }, } -pub fn subvolume(argv: Vec) -> i32 { +pub fn subvolume(argv: Vec) -> Result<()> { let cli = Cli::parse_from(argv); match cli.subcommands { @@ -47,25 +48,25 @@ pub fn subvolume(argv: Vec) -> i32 { } else { env::current_dir() .map(|p| p.join(target)) - .expect("unable to get current directory") + .context("unable to get current directory")? }; if let Some(dirname) = target.parent() { let fs = unsafe { BcachefsHandle::open(dirname) }; fs.create_subvolume(target) - .expect("Failed to create the subvolume"); + .context("Failed to create the subvolume")?; } } } Subcommands::Delete { target } => { let target = target .canonicalize() - .expect("subvolume path does not exist or can not be canonicalized"); + .context("subvolume path does not exist or can not be canonicalized")?; if let Some(dirname) = target.parent() { let fs = unsafe { BcachefsHandle::open(dirname) }; fs.delete_subvolume(target) - .expect("Failed to delete the subvolume"); + .context("Failed to delete the subvolume")?; } } Subcommands::Snapshot { @@ -91,10 +92,10 @@ pub fn subvolume(argv: Vec) -> i32 { source, dest, ) - .expect("Failed to snapshot the subvolume"); + .context("Failed to snapshot the subvolume")?; } } } - 0 + Ok(()) }