Reorganize some code

native/src/base/cxx_extern.rs

@@ -7,11 +7,10 @@ use cfg_if::cfg_if;
 use cxx::private::c_char;
 use libc::mode_t;
 
-use crate::logging::CxxResultExt;
 pub(crate) use crate::xwrap::*;
 use crate::{
-    clone_attr, cstr, fclone_attr, fd_path, map_fd, map_file, slice_from_ptr, Directory, FsPath,
+    clone_attr, cstr, fclone_attr, fd_path, map_fd, map_file, slice_from_ptr, CxxResultExt,
-    Utf8CStr, Utf8CStrBufRef,
+    Directory, FsPath, Utf8CStr, Utf8CStrBufRef,
 };
 
 pub(crate) fn fd_path_for_cxx(fd: RawFd, buf: &mut [u8]) -> isize {

native/src/base/lib.rs

@@ -11,12 +11,14 @@ use cxx_extern::*;
 pub use files::*;
 pub use logging::*;
 pub use misc::*;
+pub use result::*;
 
 mod cstr;
 mod cxx_extern;
 mod files;
 mod logging;
 mod misc;
+mod result;
 mod xwrap;
 
 #[cxx::bridge]

native/src/base/logging.rs

@@ -1,27 +1,14 @@
+use std::fmt;
+use std::fmt::Arguments;
+use std::io::{stderr, stdout, Write};
+use std::process::exit;
+
 use num_derive::{FromPrimitive, ToPrimitive};
 use num_traits::FromPrimitive;
-use std::fmt;
-use std::fmt::{Arguments, Display};
-use std::io::{stderr, stdout, Write};
-use std::panic::Location;
-use std::process::exit;
 
 use crate::ffi::LogLevelCxx;
 use crate::{Utf8CStr, Utf8CStrBufArr};
 
-// Error handling and logging throughout the Rust codebase in Magisk:
-//
-// All errors should be logged and consumed as soon as possible and converted into LoggedError.
-// For `Result` with errors that implement the `Display` trait, use the `?` operator to
-// log and convert to LoggedResult.
-//
-// To log an error with more information, use `ResultExt::log_with_msg()`.
-//
-// The "cxx" method variants in `CxxResultExt` are only used for C++ interop and
-// should not be used directly in any Rust code.
-//
-// For general logging, use the <level>!(...) macros.
-
 // Ugly hack to avoid using enum
 #[allow(non_snake_case, non_upper_case_globals)]
 mod LogFlag {
@@ -50,7 +37,7 @@ pub static mut LOGGER: Logger = Logger {
 };
 
 type LogWriter = fn(level: LogLevel, msg: &Utf8CStr);
-type Formatter<'a> = &'a mut dyn fmt::Write;
+pub(crate) type Formatter<'a> = &'a mut dyn fmt::Write;
 
 #[derive(Copy, Clone)]
 pub struct Logger {
@@ -171,188 +158,3 @@ macro_rules! debug {
 macro_rules! debug {
     ($($args:tt)+) => {};
 }
-
-#[derive(Default)]
-pub struct LoggedError {}
-
-// Automatically handle all printable errors
-impl<T: Display> From<T> for LoggedError {
-    #[cfg(not(debug_assertions))]
-    fn from(e: T) -> Self {
-        log_with_args(LogLevel::Error, format_args_nl!("{:#}", e));
-        LoggedError::default()
-    }
-
-    #[track_caller]
-    #[cfg(debug_assertions)]
-    fn from(e: T) -> Self {
-        let caller = Location::caller();
-        log_with_args(
-            LogLevel::Error,
-            format_args_nl!("[{}:{}] {:#}", caller.file(), caller.line(), e),
-        );
-        LoggedError::default()
-    }
-}
-
-pub type LoggedResult<T> = Result<T, LoggedError>;
-
-#[macro_export]
-macro_rules! log_err {
-    ($msg:literal $(,)?) => {{
-        $crate::log_with_args($crate::LogLevel::Error, format_args_nl!($msg));
-        $crate::LoggedError::default()
-    }};
-    ($err:expr $(,)?) => {{
-        $crate::log_with_args($crate::LogLevel::Error, format_args_nl!("{}", $err));
-        $crate::LoggedError::default()
-    }};
-    ($($args:tt)+) => {{
-        $crate::log_with_args($crate::LogLevel::Error, format_args_nl!($($args)+));
-        $crate::LoggedError::default()
-    }};
-}
-
-pub trait ResultExt<T> {
-    fn log(self) -> LoggedResult<T>;
-    fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T>;
-}
-
-pub trait ResultNoLog<T> {
-    fn no_log(self) -> LoggedResult<T>;
-}
-
-// Internal C++ bridging logging routines
-pub(crate) trait CxxResultExt<T> {
-    fn log_cxx(self) -> LoggedResult<T>;
-    fn log_cxx_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T>;
-}
-
-trait LogImpl<T> {
-    fn log_impl(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedResult<T>;
-    fn log_with_msg_impl<F: FnOnce(Formatter) -> fmt::Result>(
-        self,
-        level: LogLevel,
-        caller: Option<&'static Location>,
-        f: F,
-    ) -> LoggedResult<T>;
-}
-
-impl<T, E> ResultNoLog<T> for Result<T, E> {
-    fn no_log(self) -> LoggedResult<T> {
-        match self {
-            Ok(v) => Ok(v),
-            Err(_) => Err(LoggedError::default()),
-        }
-    }
-}
-
-impl<T> ResultNoLog<T> for Option<T> {
-    fn no_log(self) -> LoggedResult<T> {
-        match self {
-            Some(v) => Ok(v),
-            None => Err(LoggedError::default()),
-        }
-    }
-}
-
-impl<T, R: LogImpl<T>> CxxResultExt<T> for R {
-    fn log_cxx(self) -> LoggedResult<T> {
-        self.log_impl(LogLevel::ErrorCxx, None)
-    }
-
-    fn log_cxx_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
-        self.log_with_msg_impl(LogLevel::ErrorCxx, None, f)
-    }
-}
-
-impl<T, R: LogImpl<T>> ResultExt<T> for R {
-    #[cfg(not(debug_assertions))]
-    fn log(self) -> LoggedResult<T> {
-        self.log_impl(LogLevel::Error, None)
-    }
-
-    #[track_caller]
-    #[cfg(debug_assertions)]
-    fn log(self) -> LoggedResult<T> {
-        self.log_impl(LogLevel::Error, Some(Location::caller()))
-    }
-
-    #[cfg(not(debug_assertions))]
-    fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
-        self.log_with_msg_impl(LogLevel::Error, None, f)
-    }
-
-    #[track_caller]
-    #[cfg(debug_assertions)]
-    fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
-        self.log_with_msg_impl(LogLevel::Error, Some(Location::caller()), f)
-    }
-}
-
-impl<T> LogImpl<T> for LoggedResult<T> {
-    fn log_impl(self, _: LogLevel, _: Option<&'static Location>) -> LoggedResult<T> {
-        self
-    }
-
-    fn log_with_msg_impl<F: FnOnce(Formatter) -> fmt::Result>(
-        self,
-        level: LogLevel,
-        caller: Option<&'static Location>,
-        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())
-            }
-        }
-    }
-}
-
-impl<T, E: Display> LogImpl<T> for Result<T, E> {
-    fn log_impl(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedResult<T> {
-        match self {
-            Ok(v) => Ok(v),
-            Err(e) => {
-                if let Some(caller) = caller {
-                    log_with_args(
-                        level,
-                        format_args_nl!("[{}:{}] {:#}", caller.file(), caller.line(), e),
-                    );
-                } else {
-                    log_with_args(level, format_args_nl!("{:#}", e));
-                }
-                Err(LoggedError::default())
-            }
-        }
-    }
-
-    fn log_with_msg_impl<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())
-            }
-        }
-    }
-}

native/src/base/result.rs

@@ -0,0 +1,195 @@
+use std::fmt;
+use std::fmt::Display;
+use std::panic::Location;
+
+use crate::logging::Formatter;
+use crate::{log_with_args, log_with_formatter, LogLevel};
+
+// Error handling throughout the Rust codebase in Magisk:
+//
+// All errors should be logged and consumed as soon as possible and converted into LoggedError.
+// For `Result` with errors that implement the `Display` trait, use the `?` operator to
+// log and convert to LoggedResult.
+//
+// To log an error with more information, use `ResultExt::log_with_msg()`.
+//
+// The "cxx" method variants in `CxxResultExt` are only used for C++ interop and
+// should not be used directly in any Rust code.
+
+#[derive(Default)]
+pub struct LoggedError {}
+pub type LoggedResult<T> = Result<T, LoggedError>;
+
+#[macro_export]
+macro_rules! log_err {
+    ($($args:tt)+) => {{
+        $crate::log_with_args($crate::LogLevel::Error, format_args_nl!($($args)+));
+        $crate::LoggedError::default()
+    }};
+}
+
+// Any result or option can be silenced
+pub trait SilentResultExt<T> {
+    fn silent(self) -> LoggedResult<T>;
+}
+
+impl<T, E> SilentResultExt<T> for Result<T, E> {
+    fn silent(self) -> LoggedResult<T> {
+        match self {
+            Ok(v) => Ok(v),
+            Err(_) => Err(LoggedError::default()),
+        }
+    }
+}
+
+impl<T> SilentResultExt<T> for Option<T> {
+    fn silent(self) -> LoggedResult<T> {
+        match self {
+            Some(v) => Ok(v),
+            None => Err(LoggedError::default()),
+        }
+    }
+}
+
+// Public API for logging results
+pub trait ResultExt<T> {
+    fn log(self) -> LoggedResult<T>;
+    fn log_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T>;
+}
+
+// Internal C++ bridging logging routines
+pub(crate) trait CxxResultExt<T> {
+    fn log_cxx(self) -> LoggedResult<T>;
+    fn log_cxx_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T>;
+}
+
+trait Loggable<T> {
+    fn do_log(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedResult<T>;
+    fn do_log_msg<F: FnOnce(Formatter) -> fmt::Result>(
+        self,
+        level: LogLevel,
+        caller: Option<&'static Location>,
+        f: F,
+    ) -> LoggedResult<T>;
+}
+
+impl<T, R: Loggable<T>> CxxResultExt<T> for R {
+    fn log_cxx(self) -> LoggedResult<T> {
+        self.do_log(LogLevel::ErrorCxx, None)
+    }
+
+    fn log_cxx_with_msg<F: FnOnce(Formatter) -> fmt::Result>(self, f: F) -> LoggedResult<T> {
+        self.do_log_msg(LogLevel::ErrorCxx, None, f)
+    }
+}
+
+impl<T, R: Loggable<T>> ResultExt<T> for R {
+    #[cfg(not(debug_assertions))]
+    fn log(self) -> LoggedResult<T> {
+        self.do_log(LogLevel::Error, None)
+    }
+
+    #[track_caller]
+    #[cfg(debug_assertions)]
+    fn log(self) -> LoggedResult<T> {
+        self.do_log(LogLevel::Error, Some(Location::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)
+    }
+
+    #[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)
+    }
+}
+
+impl<T> Loggable<T> for LoggedResult<T> {
+    fn do_log(self, _: LogLevel, _: Option<&'static Location>) -> LoggedResult<T> {
+        self
+    }
+
+    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(_) => {
+                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())
+            }
+        }
+    }
+}
+
+impl<T, E: Display> Loggable<T> for Result<T, E> {
+    fn do_log(self, level: LogLevel, caller: Option<&'static Location>) -> LoggedResult<T> {
+        match self {
+            Ok(v) => Ok(v),
+            Err(e) => {
+                if let Some(caller) = caller {
+                    log_with_args(
+                        level,
+                        format_args_nl!("[{}:{}] {:#}", caller.file(), caller.line(), e),
+                    );
+                } else {
+                    log_with_args(level, format_args_nl!("{:#}", 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, format_args_nl!("{:#}", e));
+        LoggedError::default()
+    }
+
+    #[track_caller]
+    #[cfg(debug_assertions)]
+    fn from(e: T) -> Self {
+        let caller = Location::caller();
+        log_with_args(
+            LogLevel::Error,
+            format_args_nl!("[{}:{}] {:#}", caller.file(), caller.line(), e),
+        );
+        LoggedError::default()
+    }
+}

native/src/core/resetprop/persist.rs

@@ -12,7 +12,7 @@ use quick_protobuf::{BytesReader, MessageRead, MessageWrite, Writer};
 use base::libc::{O_CLOEXEC, O_RDONLY};
 use base::{
     clone_attr, cstr, debug, libc::mkstemp, Directory, FsPath, FsPathBuf, LibcReturn, LoggedResult,
-    MappedFile, ResultNoLog, Utf8CStr, Utf8CStrBufArr, WalkResult,
+    MappedFile, SilentResultExt, Utf8CStr, Utf8CStrBufArr, WalkResult,
 };
 
 use crate::ffi::{prop_cb_exec, PropCb};
@@ -67,7 +67,7 @@ impl PropExt for PersistentProperties {
     }
 
     fn find(&mut self, name: &Utf8CStr) -> LoggedResult<&mut PersistentPropertyRecord> {
-        let idx = self.find_index(name).no_log()?;
+        let idx = self.find_index(name).silent()?;
         Ok(&mut self[idx])
     }
 }
@@ -81,7 +81,7 @@ fn file_get_prop(name: &Utf8CStr) -> LoggedResult<String> {
     let path = FsPathBuf::new(&mut buf)
         .join(PERSIST_PROP_DIR!())
         .join(name);
-    let mut file = path.open(O_RDONLY | O_CLOEXEC).no_log()?;
+    let mut file = path.open(O_RDONLY | O_CLOEXEC).silent()?;
     debug!("resetprop: read prop from [{}]", path);
     let mut s = String::new();
     file.read_to_string(&mut s)?;
@@ -108,7 +108,7 @@ fn file_set_prop(name: &Utf8CStr, value: Option<&Utf8CStr>) -> LoggedResult<()>
         debug!("resetprop: write prop to [{}]", tmp);
         tmp.rename_to(path)?
     } else {
-        path.remove().no_log()?;
+        path.remove().silent()?;
         debug!("resetprop: unlink [{}]", path);
     }
     Ok(())
@@ -199,7 +199,7 @@ pub unsafe fn persist_delete_prop(name: &Utf8CStr) -> bool {
     fn inner(name: &Utf8CStr) -> LoggedResult<()> {
         if check_proto() {
             let mut props = proto_read_props()?;
-            let idx = props.find_index(name).no_log()?;
+            let idx = props.find_index(name).silent()?;
             props.remove(idx);
             proto_write_props(&props)
         } else {