Simplify ResultExt implementation

Also introduce OptionExt

native/src/base/result.rs

@@ -33,25 +33,19 @@ macro_rules! log_err {
 }
 
 // Any result or option can be silenced
-pub trait SilentResultExt<T> {
+pub trait SilentLogExt<T> {
     fn silent(self) -> LoggedResult<T>;
 }
 
-impl<T, E> SilentResultExt<T> for Result<T, E> {
+impl<T, E> SilentLogExt<T> for Result<T, E> {
     fn silent(self) -> LoggedResult<T> {
-        match self {
+        self.map_err(|_| LoggedError::default())
-            Ok(v) => Ok(v),
-            Err(_) => Err(LoggedError::default()),
-        }
     }
 }
 
-impl<T> SilentResultExt<T> for Option<T> {
+impl<T> SilentLogExt<T> for Option<T> {
     fn silent(self) -> LoggedResult<T> {
-        match self {
+        self.ok_or_else(LoggedError::default)
-            Some(v) => Ok(v),
-            None => Err(LoggedError::default()),
-        }
     }
 }
 
@@ -67,142 +61,170 @@ pub(crate) trait CxxResultExt<T> {
     fn log_cxx(self) -> LoggedResult<T>;
 }
 
-trait Loggable<T> {
+// Public API for converting Option to LoggedResult
-    fn do_log(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedResult<T>;
+pub trait OptionExt<T> {
+    fn ok_or_log(self) -> LoggedResult<T>;
+    fn ok_or_log_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T>;
+}
+
+impl<T> OptionExt<T> for Option<T> {
+    #[inline(always)]
+    fn ok_or_log(self) -> LoggedResult<T> {
+        self.ok_or_else(LoggedError::default)
+    }
+
+    #[cfg(not(debug_assertions))]
+    fn ok_or_log_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
+        self.ok_or_else(|| {
+            do_log_msg(LogLevel::Error, None, f);
+            LoggedError::default()
+        })
+    }
+
+    #[track_caller]
+    #[cfg(debug_assertions)]
+    fn ok_or_log_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
+        let caller = Some(Location::caller());
+        self.ok_or_else(|| {
+            do_log_msg(LogLevel::Error, caller, f);
+            LoggedError::default()
+        })
+    }
+}
+
+trait Loggable {
+    fn do_log(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedError;
     fn do_log_msg<F: FnOnce(Formatter) -> fmt::Result>(
         self,
         level: LogLevel,
         caller: Option<&'static Location>,
         f: F,
-    ) -> LoggedResult<T>;
+    ) -> LoggedError;
 }
 
-impl<T, R: Loggable<T>> CxxResultExt<T> for R {
+impl<T, E: Loggable> CxxResultExt<T> for Result<T, E> {
     fn log_cxx(self) -> LoggedResult<T> {
-        self.do_log(LogLevel::ErrorCxx, None)
+        self.map_err(|e| e.do_log(LogLevel::ErrorCxx, None))
     }
 }
 
-impl<T, R: Loggable<T>> ResultExt<T> for R {
+impl<T, E: Loggable> ResultExt<T> for Result<T, E> {
     #[cfg(not(debug_assertions))]
     fn log(self) -> LoggedResult<T> {
-        self.do_log(LogLevel::Error, None)
+        self.map_err(|e| e.do_log(LogLevel::Error, None))
     }
 
     #[track_caller]
     #[cfg(debug_assertions)]
     fn log(self) -> LoggedResult<T> {
-        self.do_log(LogLevel::Error, Some(Location::caller()))
+        let caller = Some(Location::caller());
+        self.map_err(|e| e.do_log(LogLevel::Error, caller))
     }
 
     #[cfg(not(debug_assertions))]
     fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
-        self.do_log_msg(LogLevel::Error, None, f)
+        self.map_err(|e| e.do_log_msg(LogLevel::Error, None, f))
     }
 
     #[track_caller]
     #[cfg(debug_assertions)]
     fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
-        self.do_log_msg(LogLevel::Error, Some(Location::caller()), f)
+        let caller = Some(Location::caller());
+        self.map_err(|e| e.do_log_msg(LogLevel::Error, caller, f))
     }
 
     #[cfg(not(debug_assertions))]
     fn log_ok(self) {
-        self.log().ok();
+        self.map_err(|e| e.do_log(LogLevel::Error, None)).ok();
     }
 
     #[track_caller]
     #[cfg(debug_assertions)]
     fn log_ok(self) {
-        self.do_log(LogLevel::Error, Some(Location::caller())).ok();
+        let caller = Some(Location::caller());
+        self.map_err(|e| e.do_log(LogLevel::Error, caller)).ok();
     }
 }
 
-impl<T> Loggable<T> for LoggedResult<T> {
+impl<T> ResultExt<T> for LoggedResult<T> {
-    fn do_log(self, _: LogLevel, _: Option<&'static Location>) -> LoggedResult<T> {
+    fn log(self) -> LoggedResult<T> {
         self
     }
 
-    fn do_log_msg<F: FnOnce(Formatter) -> fmt::Result>(
+    #[cfg(not(debug_assertions))]
-        self,
+    fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
-        level: LogLevel,
+        do_log_msg(LogLevel::Error, None, f);
-        caller: Option<&'static Location>,
+        self
-        f: F,
-    ) -> LoggedResult<T> {
-        match self {
-            Ok(v) => Ok(v),
-            Err(_) => {
-                log_with_formatter(level, |w| {
-                    if let Some(caller) = caller {
-                        write!(w, "[{}:{}] ", caller.file(), caller.line())?;
-                    }
-                    f(w)?;
-                    w.write_char('\n')
-                });
-                Err(LoggedError::default())
-            }
-        }
     }
+
+    #[track_caller]
+    #[cfg(debug_assertions)]
+    fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
+        let caller = Some(Location::caller());
+        do_log_msg(LogLevel::Error, caller, f);
+        self
+    }
+
+    fn log_ok(self) {}
 }
 
-impl<T, E: Display> Loggable<T> for Result<T, E> {
+// Allow converting Loggable errors to LoggedError to support `?` operator
-    fn do_log(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedResult<T> {
+impl<T: Loggable> From<T> for LoggedError {
-        match self {
-            Ok(v) => Ok(v),
-            Err(e) => {
-                if let Some(caller) = caller {
-                    log_with_args!(level, "[{}:{}] {:#}", caller.file(), caller.line(), e);
-                } else {
-                    log_with_args!(level, "{:#}", e);
-                }
-                Err(LoggedError::default())
-            }
-        }
-    }
-
-    fn do_log_msg<F: FnOnce(Formatter) -> fmt::Result>(
-        self,
-        level: LogLevel,
-        caller: Option<&'static Location>,
-        f: F,
-    ) -> LoggedResult<T> {
-        match self {
-            Ok(v) => Ok(v),
-            Err(e) => {
-                log_with_formatter(level, |w| {
-                    if let Some(caller) = caller {
-                        write!(w, "[{}:{}] ", caller.file(), caller.line())?;
-                    }
-                    f(w)?;
-                    writeln!(w, ": {e:#}")
-                });
-                Err(LoggedError::default())
-            }
-        }
-    }
-}
-
-// Automatically convert all printable errors to LoggedError to support `?` operator
-impl<T: Display> From<T> for LoggedError {
     #[cfg(not(debug_assertions))]
     fn from(e: T) -> Self {
-        log_with_args!(LogLevel::Error, "{:#}", e);
+        e.do_log(LogLevel::Error, None)
-        LoggedError::default()
     }
 
     #[track_caller]
     #[cfg(debug_assertions)]
     fn from(e: T) -> Self {
-        let caller = Location::caller();
+        let caller = Some(Location::caller());
-        log_with_args!(
+        e.do_log(LogLevel::Error, caller)
-            LogLevel::Error,
+    }
-            "[{}:{}] {:#}",
+}
-            caller.file(),
+
-            caller.line(),
+// Actual logging implementation
-            e
+
-        );
+// Make all printable objects Loggable
+impl<T: Display> Loggable for T {
+    fn do_log(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedError {
+        if let Some(caller) = caller {
+            log_with_args!(level, "[{}:{}] {:#}", caller.file(), caller.line(), self);
+        } else {
+            log_with_args!(level, "{:#}", self);
+        }
         LoggedError::default()
     }
+
+    fn do_log_msg<F: FnOnce(Formatter) -> fmt::Result>(
+        self,
+        level: LogLevel,
+        caller: Option<&'static Location>,
+        f: F,
+    ) -> LoggedError {
+        log_with_formatter(level, |w| {
+            if let Some(caller) = caller {
+                write!(w, "[{}:{}] ", caller.file(), caller.line())?;
+            }
+            f(w)?;
+            writeln!(w, ": {self:#}")
+        });
+        LoggedError::default()
+    }
+}
+
+fn do_log_msg<F: FnOnce(Formatter) -> fmt::Result>(
+    level: LogLevel,
+    caller: Option<&'static Location>,
+    f: F,
+) {
+    log_with_formatter(level, |w| {
+        if let Some(caller) = caller {
+            write!(w, "[{}:{}] ", caller.file(), caller.line())?;
+        }
+        f(w)?;
+        w.write_char('\n')
+    });
 }
 
 // Check libc return value and map to Result

native/src/boot/cpio.rs

@@ -20,8 +20,8 @@ use base::libc::{
     dev_t, gid_t, major, makedev, minor, mknod, mode_t, uid_t,
 };
 use base::{
-    BytesExt, EarlyExitExt, LoggedError, LoggedResult, MappedFile, ResultExt, Utf8CStr,
+    BytesExt, EarlyExitExt, LoggedResult, MappedFile, OptionExt, ResultExt, Utf8CStr, Utf8CStrBuf,
-    Utf8CStrBuf, WriteExt, cstr, error, log_err,
+    WriteExt, cstr, log_err,
 };
 
 use crate::check_env;
@@ -328,10 +328,10 @@ impl Cpio {
     }
 
     fn extract_entry(&self, path: &str, out: &mut String) -> LoggedResult<()> {
-        let entry = self.entries.get(path).ok_or_else(|| {
+        let entry = self
-            error!("No such file");
+            .entries
-            LoggedError::default()
+            .get(path)
-        })?;
+            .ok_or_log_msg(|w| w.write_str("No such file"))?;
         eprintln!("Extracting entry [{path}] to [{out}]");
 
         let out = Utf8CStr::from_string(out);
@@ -462,10 +462,10 @@ impl Cpio {
     }
 
     fn mv(&mut self, from: &str, to: &str) -> LoggedResult<()> {
-        let entry = self.entries.remove(&norm_path(from)).ok_or_else(|| {
+        let entry = self
-            error!("no such entry {}", from);
+            .entries
-            LoggedError::default()
+            .remove(&norm_path(from))
-        })?;
+            .ok_or_log_msg(|w| w.write_fmt(format_args!("No such entry {from}")))?;
         self.entries.insert(norm_path(to), entry);
         eprintln!("Move [{from}] -> [{to}]");
         Ok(())
@@ -813,10 +813,8 @@ fn x8u(x: &[u8; 8]) -> LoggedResult<u32> {
     let s = str::from_utf8(x).log_with_msg(|w| w.write_str("bad cpio header"))?;
     for c in s.chars() {
         ret = ret * 16
-            + c.to_digit(16).ok_or_else(|| {
+            + c.to_digit(16)
-                error!("bad cpio header");
+                .ok_or_log_msg(|w| w.write_str("bad cpio header"))?;
-                LoggedError::default()
-            })?;
     }
     Ok(ret)
 }

native/src/core/module.rs

@@ -4,8 +4,8 @@ use crate::ffi::{ModuleInfo, exec_module_scripts, exec_script, get_magisk_tmp, l
 use crate::mount::setup_module_mount;
 use base::{
     DirEntry, Directory, FsPathBuilder, LibcReturn, LoggedResult, OsResultStatic, ResultExt,
-    SilentResultExt, Utf8CStr, Utf8CStrBuf, Utf8CString, WalkResult, clone_attr, cstr, debug,
+    SilentLogExt, Utf8CStr, Utf8CStrBuf, Utf8CString, WalkResult, clone_attr, cstr, debug, error,
-    error, info, libc, raw_cstr, warn,
+    info, libc, raw_cstr, warn,
 };
 use libc::{AT_REMOVEDIR, MS_RDONLY, O_CLOEXEC, O_CREAT, O_RDONLY};
 use std::collections::BTreeMap;

native/src/core/resetprop/persist.rs

@@ -16,7 +16,7 @@ use crate::resetprop::proto::persistent_properties::{
 use base::const_format::concatcp;
 use base::libc::{O_CLOEXEC, O_RDONLY};
 use base::{
-    Directory, FsPathBuilder, LibcReturn, LoggedResult, MappedFile, SilentResultExt, Utf8CStr,
+    Directory, FsPathBuilder, LibcReturn, LoggedResult, MappedFile, SilentLogExt, Utf8CStr,
     Utf8CStrBuf, WalkResult, clone_attr, cstr, debug, libc::mkstemp,
 };
 

native/src/init/logging.rs

@@ -1,5 +1,5 @@
 use base::{
-    LOGGER, LogLevel, Logger, SilentResultExt, Utf8CStr, cstr,
+    LOGGER, LogLevel, Logger, SilentLogExt, Utf8CStr, cstr,
     libc::{
         O_CLOEXEC, O_RDWR, O_WRONLY, S_IFCHR, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO, SYS_dup3,
         makedev, mknod, syscall,