Skip to content

Commit

Permalink
Organize logging code
Browse files Browse the repository at this point in the history
  • Loading branch information
topjohnwu committed Sep 19, 2023
1 parent b750c89 commit 15e13a8
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 27 deletions.
3 changes: 2 additions & 1 deletion native/src/base/cxx_extern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ use std::os::fd::{BorrowedFd, OwnedFd, RawFd};
use cxx::private::c_char;
use libc::mode_t;

use crate::logging::CxxResultExt;
pub(crate) use crate::xwrap::*;
use crate::{fd_path, map_fd, map_file, Directory, FsPath, ResultExt, Utf8CStr, Utf8CStrSlice};
use crate::{fd_path, map_fd, map_file, Directory, FsPath, Utf8CStr, Utf8CStrSlice};

pub(crate) fn fd_path_for_cxx(fd: RawFd, buf: &mut [u8]) -> isize {
let mut buf = Utf8CStrSlice::from(buf);
Expand Down
62 changes: 37 additions & 25 deletions native/src/base/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use crate::{Utf8CStr, Utf8CStrArr};
// 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.
// Implement `From<ErrorType> for LoggedError` for non-standard error types so that we can
// directly use the `?` operator to propagate LoggedResult.
// 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 `ResultExt` are only used for C++ interop and
// 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.
Expand Down Expand Up @@ -202,10 +202,38 @@ macro_rules! log_err {
}};
}

pub trait ResultExt<T>
where
Self: Sized,
{
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 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, 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)
Expand All @@ -227,25 +255,9 @@ where
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)
}

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)
}

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> ResultExt<T> for LoggedResult<T> {
impl<T> LogImpl<T> for LoggedResult<T> {
fn log_impl(self, _: LogLevel, _: Option<&'static Location>) -> LoggedResult<T> {
self
}
Expand All @@ -272,7 +284,7 @@ impl<T> ResultExt<T> for LoggedResult<T> {
}
}

impl<T, E: Display> ResultExt<T> for Result<T, E> {
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),
Expand Down
2 changes: 1 addition & 1 deletion native/src/base/xwrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use libc::{
ssize_t, SYS_dup3,
};

use crate::{cstr, errno, raw_cstr, FsPath, ResultExt, Utf8CStr, Utf8CStrSlice};
use crate::{cstr, errno, raw_cstr, CxxResultExt, FsPath, Utf8CStr, Utf8CStrSlice};

fn ptr_to_str<'a, T>(ptr: *const T) -> &'a str {
if ptr.is_null() {
Expand Down

0 comments on commit 15e13a8

Please sign in to comment.