Standardize logging and error handling

- Introduce new types: LoggedResult and LoggedError
- Introduce new extension methods to log and add additional msgs
- Never exit when logging error messages in Rust (all errors should be
  handled by using Result and Rust's error propagation)
- Remove all usages of anyhow as it doesn't fit Magisk's use cases
This commit is contained in:
topjohnwu
2023-06-29 16:44:44 -07:00
parent dbc2236dd2
commit 4ee4cbada6
16 changed files with 254 additions and 145 deletions

View File

@@ -15,7 +15,6 @@ pb-rs = { workspace = true }
base = { path = "../base" }
cxx = { path = "../external/cxx-rs" }
byteorder = { workspace = true }
anyhow = { workspace = true }
size = { workspace = true }
quick-protobuf = { workspace = true }
argh = { workspace = true }

View File

@@ -1,6 +1,6 @@
use std::collections::BTreeMap;
use std::ffi::CStr;
use std::fmt::{Display, Formatter};
use std::fmt::{Display, Formatter, Write as FmtWrite};
use std::fs::{metadata, read, DirBuilder, File};
use std::io::Write;
use std::mem::size_of;
@@ -9,16 +9,15 @@ use std::path::Path;
use std::process::exit;
use std::slice;
use anyhow::{anyhow, Context};
use argh::{EarlyExit, FromArgs};
use size::{Base, Size, Style};
use argh::{EarlyExit, FromArgs};
use base::libc::{
c_char, dev_t, gid_t, major, makedev, minor, mknod, mode_t, uid_t, S_IFBLK, S_IFCHR, S_IFDIR,
S_IFLNK, S_IFMT, S_IFREG, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR, S_IXGRP,
S_IXOTH, S_IXUSR,
};
use base::{MappedFile, ResultExt, StrErr, Utf8CStr, WriteExt};
use base::{log_err, LoggedResult, MappedFile, ResultExt, StrErr, Utf8CStr, WriteExt};
use crate::ramdisk::MagiskCpio;
@@ -217,13 +216,13 @@ impl Cpio {
}
}
fn load_from_data(data: &[u8]) -> anyhow::Result<Self> {
fn load_from_data(data: &[u8]) -> LoggedResult<Self> {
let mut cpio = Cpio::new();
let mut pos = 0usize;
while pos < data.len() {
let hdr = unsafe { &*(data.as_ptr().add(pos) as *const CpioHeader) };
if &hdr.magic != b"070701" {
return Err(anyhow!("invalid cpio magic"));
return Err(log_err!("invalid cpio magic"));
}
pos += size_of::<CpioHeader>();
let name = CStr::from_bytes_until_nul(&data[pos..])?
@@ -257,13 +256,13 @@ impl Cpio {
Ok(cpio)
}
pub(crate) fn load_from_file(path: &Utf8CStr) -> anyhow::Result<Self> {
pub(crate) fn load_from_file(path: &Utf8CStr) -> LoggedResult<Self> {
eprintln!("Loading cpio: [{}]", path);
let file = MappedFile::open(path)?;
Self::load_from_data(file.as_ref())
}
fn dump(&self, path: &str) -> anyhow::Result<()> {
fn dump(&self, path: &str) -> LoggedResult<()> {
eprintln!("Dumping cpio: [{}]", path);
let mut file = File::create(path)?;
let mut pos = 0usize;
@@ -324,8 +323,8 @@ impl Cpio {
}
}
fn extract_entry(&self, path: &str, out: &Path) -> anyhow::Result<()> {
let entry = self.entries.get(path).ok_or(anyhow!("No such file"))?;
fn extract_entry(&self, path: &str, out: &Path) -> LoggedResult<()> {
let entry = self.entries.get(path).ok_or(log_err!("No such file"))?;
eprintln!("Extracting entry [{}] to [{}]", path, out.to_string_lossy());
if let Some(parent) = out.parent() {
DirBuilder::new()
@@ -358,13 +357,13 @@ impl Cpio {
};
}
_ => {
return Err(anyhow!("unknown entry type"));
return Err(log_err!("unknown entry type"));
}
}
Ok(())
}
fn extract(&self, path: Option<&str>, out: Option<&str>) -> anyhow::Result<()> {
fn extract(&self, path: Option<&str>, out: Option<&str>) -> LoggedResult<()> {
let path = path.map(norm_path);
let out = out.map(Path::new);
if let (Some(path), Some(out)) = (&path, &out) {
@@ -384,9 +383,9 @@ impl Cpio {
self.entries.contains_key(&norm_path(path))
}
fn add(&mut self, mode: &mode_t, path: &str, file: &str) -> anyhow::Result<()> {
fn add(&mut self, mode: &mode_t, path: &str, file: &str) -> LoggedResult<()> {
if path.ends_with('/') {
return Err(anyhow!("path cannot end with / for add"));
return Err(log_err!("path cannot end with / for add"));
}
let file = Path::new(file);
let content = read(file)?;
@@ -403,7 +402,7 @@ impl Cpio {
} else if metadata.file_type().is_char_device() {
mode | S_IFCHR
} else {
return Err(anyhow!("unsupported file type"));
return Err(log_err!("unsupported file type"));
}
};
self.entries.insert(
@@ -451,11 +450,11 @@ impl Cpio {
eprintln!("Create symlink [{}] -> [{}]", dst, src);
}
fn mv(&mut self, from: &str, to: &str) -> anyhow::Result<()> {
fn mv(&mut self, from: &str, to: &str) -> LoggedResult<()> {
let entry = self
.entries
.remove(&norm_path(from))
.ok_or(anyhow!("no such entry {}", from))?;
.ok_or(log_err!("no such entry {}", from))?;
self.entries.insert(norm_path(to), entry);
eprintln!("Move [{}] -> [{}]", from, to);
Ok(())
@@ -521,9 +520,9 @@ impl Display for CpioEntry {
}
pub fn cpio_commands(argc: i32, argv: *const *const c_char) -> bool {
fn inner(argc: i32, argv: *const *const c_char) -> anyhow::Result<()> {
fn inner(argc: i32, argv: *const *const c_char) -> LoggedResult<()> {
if argc < 1 {
return Err(anyhow!("no arguments"));
return Err(log_err!("no arguments"));
}
let cmds: Result<Vec<&Utf8CStr>, StrErr> =
@@ -556,7 +555,7 @@ pub fn cpio_commands(argc: i32, argv: *const *const c_char) -> bool {
eprintln!("{}", output);
exit(0)
}
Err(_) => return Err(anyhow!(output)),
Err(_) => return Err(log_err!(output)),
},
};
match &mut cli.command {
@@ -580,7 +579,7 @@ pub fn cpio_commands(argc: i32, argv: *const *const c_char) -> bool {
CpioCommands::Add(Add { mode, path, file }) => cpio.add(mode, path, file)?,
CpioCommands::Extract(Extract { paths }) => {
if !paths.is_empty() && paths.len() != 2 {
return Err(anyhow!("invalid arguments"));
return Err(log_err!("invalid arguments"));
}
cpio.extract(
paths.get(0).map(|x| x.as_str()),
@@ -597,20 +596,19 @@ pub fn cpio_commands(argc: i32, argv: *const *const c_char) -> bool {
Ok(())
}
inner(argc, argv)
.context("Failed to process cpio")
.log()
.log_with_msg(|w| w.write_str("Failed to process cpio"))
.is_ok()
}
fn x8u<U: TryFrom<u32>>(x: &[u8; 8]) -> anyhow::Result<U> {
fn x8u<U: TryFrom<u32>>(x: &[u8; 8]) -> LoggedResult<U> {
// parse hex
let mut ret = 0u32;
for i in x {
let c = *i as char;
let v = c.to_digit(16).ok_or(anyhow!("bad cpio header"))?;
let v = c.to_digit(16).ok_or(log_err!("bad cpio header"))?;
ret = ret * 16 + v;
}
ret.try_into().map_err(|_| anyhow!("bad cpio header"))
ret.try_into().map_err(|_| log_err!("bad cpio header"))
}
#[inline(always)]

View File

@@ -1,4 +1,4 @@
use base::{MappedFile, MutBytesExt, ResultExt, Utf8CStr};
use base::{LoggedResult, MappedFile, MutBytesExt, Utf8CStr};
// SAFETY: assert(buf.len() >= 1) && assert(len <= buf.len())
macro_rules! match_patterns {
@@ -102,7 +102,7 @@ fn hex2byte(hex: &[u8]) -> Vec<u8> {
}
pub fn hexpatch(file: &[u8], from: &[u8], to: &[u8]) -> bool {
fn inner(file: &[u8], from: &[u8], to: &[u8]) -> anyhow::Result<bool> {
fn inner(file: &[u8], from: &[u8], to: &[u8]) -> LoggedResult<bool> {
let file = Utf8CStr::from_bytes(file)?;
let from = Utf8CStr::from_bytes(from)?;
let to = Utf8CStr::from_bytes(to)?;
@@ -118,5 +118,5 @@ pub fn hexpatch(file: &[u8], from: &[u8], to: &[u8]) -> bool {
Ok(!v.is_empty())
}
inner(file, from, to).log().unwrap_or(false)
inner(file, from, to).unwrap_or(false)
}

View File

@@ -1,13 +1,13 @@
use std::fmt::Write as FmtWrite;
use std::fs::File;
use std::io::{BufReader, Read, Seek, SeekFrom, Write};
use std::os::fd::{AsRawFd, FromRawFd};
use anyhow::{anyhow, Context};
use byteorder::{BigEndian, ReadBytesExt};
use quick_protobuf::{BytesReader, MessageRead};
use base::libc::c_char;
use base::{ReadSeekExt, StrErr, Utf8CStr};
use base::{error, LoggedError, LoggedResult, ReadSeekExt, StrErr, Utf8CStr};
use base::{ResultExt, WriteExt};
use crate::ffi;
@@ -15,12 +15,14 @@ use crate::proto::update_metadata::mod_InstallOperation::Type;
use crate::proto::update_metadata::DeltaArchiveManifest;
macro_rules! bad_payload {
($msg:literal) => {
anyhow!(concat!("invalid payload: ", $msg))
};
($($args:tt)*) => {
anyhow!("invalid payload: {}", format_args!($($args)*))
};
($msg:literal) => {{
error!(concat!("Invalid payload: ", $msg));
LoggedError::default()
}};
($($args:tt)*) => {{
error!("Invalid payload: {}", format_args!($($args)*));
LoggedError::default()
}};
}
const PAYLOAD_MAGIC: &str = "CrAU";
@@ -29,11 +31,11 @@ fn do_extract_boot_from_payload(
in_path: &Utf8CStr,
partition_name: Option<&Utf8CStr>,
out_path: Option<&Utf8CStr>,
) -> anyhow::Result<()> {
) -> LoggedResult<()> {
let mut reader = BufReader::new(if in_path == "-" {
unsafe { File::from_raw_fd(0) }
} else {
File::open(in_path).with_context(|| format!("cannot open '{in_path}'"))?
File::open(in_path).log_with_msg(|w| write!(w, "Cannot open '{}'", in_path))?
});
let buf = &mut [0u8; 4];
@@ -88,13 +90,13 @@ fn do_extract_boot_from_payload(
.iter()
.find(|p| p.partition_name == "boot"),
};
boot.ok_or(anyhow!("boot partition not found"))?
boot.ok_or_else(|| bad_payload!("boot partition not found"))?
}
Some(name) => manifest
.partitions
.iter()
.find(|p| p.partition_name.as_str() == name)
.ok_or(anyhow!("partition '{name}' not found"))?,
.ok_or_else(|| bad_payload!("partition '{}' not found", name))?,
};
let out_str: String;
@@ -107,7 +109,7 @@ fn do_extract_boot_from_payload(
};
let mut out_file =
File::create(out_path).with_context(|| format!("cannot write to '{out_path}'"))?;
File::create(out_path).log_with_msg(|w| write!(w, "Cannot write to '{}'", out_path))?;
// Skip the manifest signature
reader.skip(manifest_sig_len as usize)?;
@@ -121,11 +123,11 @@ fn do_extract_boot_from_payload(
for operation in operations.iter() {
let data_len = operation
.data_length
.ok_or(bad_payload!("data length not found"))? as usize;
.ok_or_else(|| bad_payload!("data length not found"))? as usize;
let data_offset = operation
.data_offset
.ok_or(bad_payload!("data offset not found"))?;
.ok_or_else(|| bad_payload!("data offset not found"))?;
let data_type = operation.type_pb;
@@ -141,9 +143,9 @@ fn do_extract_boot_from_payload(
let out_offset = operation
.dst_extents
.get(0)
.ok_or(bad_payload!("dst extents not found"))?
.ok_or_else(|| bad_payload!("dst extents not found"))?
.start_block
.ok_or(bad_payload!("start block not found"))?
.ok_or_else(|| bad_payload!("start block not found"))?
* block_size;
match data_type {
@@ -184,7 +186,7 @@ pub fn extract_boot_from_payload(
in_path: *const c_char,
partition: *const c_char,
out_path: *const c_char,
) -> anyhow::Result<()> {
) -> LoggedResult<()> {
let in_path = unsafe { Utf8CStr::from_ptr(in_path) }?;
let partition = match unsafe { Utf8CStr::from_ptr(partition) } {
Ok(s) => Some(s),
@@ -197,8 +199,7 @@ pub fn extract_boot_from_payload(
Err(e) => Err(e)?,
};
do_extract_boot_from_payload(in_path, partition, out_path)
.context("Failed to extract from payload")?;
Ok(())
.log_with_msg(|w| w.write_str("Failed to extract from payload"))
}
inner(in_path, partition, out_path).log().is_ok()
inner(in_path, partition, out_path).is_ok()
}

View File

@@ -4,7 +4,7 @@ use std::env;
use std::str::from_utf8;
use base::libc::{S_IFDIR, S_IFMT, S_IFREG};
use base::Utf8CStr;
use base::{LoggedResult, Utf8CStr};
use crate::cpio::{Cpio, CpioEntry};
use crate::patch::{patch_encryption, patch_verity};
@@ -12,8 +12,8 @@ use crate::patch::{patch_encryption, patch_verity};
pub trait MagiskCpio {
fn patch(&mut self);
fn test(&self) -> i32;
fn restore(&mut self) -> anyhow::Result<()>;
fn backup(&mut self, origin: &Utf8CStr) -> anyhow::Result<()>;
fn restore(&mut self) -> LoggedResult<()>;
fn backup(&mut self, origin: &Utf8CStr) -> LoggedResult<()>;
}
const MAGISK_PATCHED: i32 = 1 << 0;
@@ -89,7 +89,7 @@ impl MagiskCpio for Cpio {
ret
}
fn restore(&mut self) -> anyhow::Result<()> {
fn restore(&mut self) -> LoggedResult<()> {
let mut backups = HashMap::<String, Box<CpioEntry>>::new();
let mut rm_list = String::new();
self.entries
@@ -120,7 +120,7 @@ impl MagiskCpio for Cpio {
Ok(())
}
fn backup(&mut self, origin: &Utf8CStr) -> anyhow::Result<()> {
fn backup(&mut self, origin: &Utf8CStr) -> LoggedResult<()> {
let mut backups = HashMap::<String, Box<CpioEntry>>::new();
let mut rm_list = String::new();
backups.insert(