Ban all unwrap usage in codebase

This commit is contained in:
topjohnwu
2025-12-08 17:35:52 -08:00
committed by John Wu
parent 200665c48a
commit ff4ca74cfe
24 changed files with 138 additions and 89 deletions

View File

@@ -53,6 +53,9 @@ pb-rs = { git = "https://github.com/topjohnwu/quick-protobuf.git" }
quick-protobuf = { git = "https://github.com/topjohnwu/quick-protobuf.git" }
lz4-sys = { path = "external/lz4-sys" }
[workspace.lints.clippy]
unwrap_used = "deny"
[profile.dev]
opt-level = "z"
lto = "thin"

View File

@@ -9,6 +9,9 @@ path = "lib.rs"
[features]
selinux = []
[lints]
workspace = true
[build-dependencies]
cxx-gen = { workspace = true }

View File

@@ -132,13 +132,9 @@ impl PositionalArgParser<'_> {
}
fn ensure_end(&mut self) -> Result<(), EarlyExit> {
if self.0.len() == 0 {
Ok(())
} else {
Err(EarlyExit::from(format!(
"Unrecognized argument: {}\n",
self.0.next().unwrap()
)))
match self.0.next() {
None => Ok(()),
Some(s) => Err(EarlyExit::from(format!("Unrecognized argument: {s}\n"))),
}
}
}

View File

@@ -7,6 +7,9 @@ edition.workspace = true
crate-type = ["staticlib"]
path = "lib.rs"
[lints]
workspace = true
[build-dependencies]
cxx-gen = { workspace = true }
pb-rs = { workspace = true }

View File

@@ -6,6 +6,7 @@ use crate::codegen::gen_cxx_binding;
#[path = "../include/codegen.rs"]
mod codegen;
#[allow(clippy::unwrap_used)]
fn main() {
println!("cargo:rerun-if-changed=proto/update_metadata.proto");

View File

@@ -248,7 +248,7 @@ pub fn get_encoder<'a, W: Write + 'a>(
FileFormat::ZOPFLI => {
// These options are already better than gzip -9
let opt = ZopfliOptions {
iteration_count: NonZeroU64::new(1).unwrap(),
iteration_count: unsafe { NonZeroU64::new_unchecked(1) },
maximum_block_splits: 1,
..Default::default()
};

View File

@@ -484,10 +484,9 @@ impl Cpio {
};
for (name, entry) in &self.entries {
let p = "/".to_string() + name.as_str();
if !p.starts_with(&path) {
let Some(p) = p.strip_prefix(&path) else {
continue;
}
let p = p.strip_prefix(&path).unwrap();
};
if !p.is_empty() && !p.starts_with('/') {
continue;
}
@@ -614,8 +613,11 @@ impl Cpio {
o.rm(".backup", true);
self.rm(".backup", true);
let mut lhs = o.entries.into_iter().peekable();
let mut rhs = self.entries.iter().peekable();
let mut left_iter = o.entries.into_iter();
let mut right_iter = self.entries.iter();
let mut lhs = left_iter.next();
let mut rhs = right_iter.next();
loop {
enum Action<'a> {
@@ -623,32 +625,38 @@ impl Cpio {
Record(&'a String),
Noop,
}
let action = match (lhs.peek(), rhs.peek()) {
(Some((l, _)), Some((r, re))) => match l.as_str().cmp(r.as_str()) {
// Move the iterator forward if needed
if lhs.is_none() {
lhs = left_iter.next();
}
if rhs.is_none() {
rhs = right_iter.next();
}
let action = match (lhs.take(), rhs.take()) {
(Some((ln, le)), Some((rn, re))) => match ln.as_str().cmp(rn.as_str()) {
Ordering::Less => {
let (l, le) = lhs.next().unwrap();
Action::Backup(l, le)
// Put rhs back
rhs = Some((rn, re));
Action::Backup(ln, le)
}
Ordering::Greater => {
// Put lhs back
lhs = Some((ln, le));
Action::Record(rn)
}
Ordering::Greater => Action::Record(rhs.next().unwrap().0),
Ordering::Equal => {
let (l, le) = lhs.next().unwrap();
let action = if re.data != le.data {
Action::Backup(l, le)
if re.data != le.data {
Action::Backup(ln, le)
} else {
Action::Noop
};
rhs.next();
action
}
}
},
(Some(_), None) => {
let (l, le) = lhs.next().unwrap();
Action::Backup(l, le)
}
(None, Some(_)) => Action::Record(rhs.next().unwrap().0),
(None, None) => {
break;
}
(Some((ln, le)), None) => Action::Backup(ln, le),
(None, Some((rn, _))) => Action::Record(rn),
(None, None) => break,
};
match action {
Action::Backup(name, mut entry) => {

View File

@@ -15,6 +15,9 @@ check-signature = []
check-client = []
su-check-db = []
[lints]
workspace = true
[build-dependencies]
cxx-gen = { workspace = true }
pb-rs = { workspace = true }

View File

@@ -82,7 +82,7 @@ impl MagiskD {
Command::new(&tmp_bb)
.arg("--install")
.arg("-s")
.arg(tmp_bb.parent_dir().unwrap())
.arg(tmp_bb.parent_dir().unwrap_or_default())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
@@ -186,13 +186,13 @@ impl MagiskD {
setup_preinit_dir();
self.ensure_manager();
if self.zygisk_enabled.load(Ordering::Relaxed) {
self.zygisk.lock().unwrap().reset(true);
self.zygisk.lock().reset(true);
}
}
pub fn boot_stage_handler(&self, client: UnixStream, code: RequestCode) {
// Make sure boot stage execution is always serialized
let mut state = self.boot_stage_lock.lock().unwrap();
let mut state = self.boot_stage_lock.lock();
match code {
RequestCode::POST_FS_DATA => {

View File

@@ -6,6 +6,7 @@ use crate::codegen::gen_cxx_binding;
#[path = "../include/codegen.rs"]
mod codegen;
#[allow(clippy::unwrap_used)]
fn main() {
println!("cargo:rerun-if-changed=resetprop/proto/persistent_properties.proto");

View File

@@ -31,8 +31,9 @@ use std::io::{BufReader, Write};
use std::os::fd::{AsFd, AsRawFd, IntoRawFd, RawFd};
use std::os::unix::net::{UCred, UnixListener, UnixStream};
use std::process::{Command, exit};
use std::sync::OnceLock;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Mutex, OnceLock};
use std::sync::nonpoison::Mutex;
use std::time::Duration;
// Global magiskd singleton
@@ -105,7 +106,7 @@ impl MagiskD {
denylist_handler(-1);
// Restore native bridge property
self.zygisk.lock().unwrap().restore_prop();
self.zygisk.lock().restore_prop();
client.write_pod(&0).log_ok();
@@ -129,7 +130,7 @@ impl MagiskD {
self.prune_su_access();
scan_deny_apps();
if self.zygisk_enabled.load(Ordering::Relaxed) {
self.zygisk.lock().unwrap().reset(false);
self.zygisk.lock().reset(false);
}
}
RequestCode::SQLITE_CMD => {

View File

@@ -187,7 +187,7 @@ unsafe extern "C" fn read_db_row<T: SqlTable>(
impl MagiskD {
fn with_db<F: FnOnce(*mut sqlite3) -> i32>(&self, f: F) -> i32 {
let mut db = self.sql_connection.lock().unwrap();
let mut db = self.sql_connection.lock();
if db.is_none() {
let raw_db = open_and_init_db();
*db = NonNull::new(raw_db).map(Sqlite3);

View File

@@ -4,6 +4,9 @@
#![feature(unix_socket_peek)]
#![feature(default_field_values)]
#![feature(peer_credentials_unix_socket)]
#![feature(sync_nonpoison)]
#![feature(nonpoison_mutex)]
#![feature(nonpoison_condvar)]
#![allow(clippy::missing_safety_doc)]
use crate::ffi::SuRequest;

View File

@@ -20,9 +20,10 @@ use std::io::{IoSlice, Read, Write};
use std::mem::ManuallyDrop;
use std::os::fd::{FromRawFd, IntoRawFd, RawFd};
use std::ptr::null_mut;
use std::sync::Arc;
use std::sync::atomic::{AtomicI32, Ordering};
use std::sync::{Arc, Mutex};
use std::time::{SystemTime, UNIX_EPOCH};
use std::sync::nonpoison::Mutex;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use std::{fs, io};
#[allow(dead_code, non_camel_case_types)]
@@ -117,12 +118,12 @@ fn write_log_to_pipe(mut logd: &File, prio: i32, msg: &Utf8CStr) -> io::Result<u
static MAGISK_LOGD_FD: Mutex<Option<Arc<File>>> = Mutex::new(None);
fn with_logd_fd<R, F: FnOnce(&File) -> io::Result<R>>(f: F) {
let fd = MAGISK_LOGD_FD.lock().unwrap().clone();
let fd = MAGISK_LOGD_FD.lock().clone();
if let Some(logd) = fd
&& f(&logd).is_err()
{
// If any error occurs, shut down the logd pipe
*MAGISK_LOGD_FD.lock().unwrap() = None;
*MAGISK_LOGD_FD.lock() = None;
}
}
@@ -265,7 +266,9 @@ fn logfile_write_loop(mut pipe: File) -> io::Result<()> {
_ => continue,
};
let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or(Duration::ZERO);
// Note: the obvious better implementation is to use the rust chrono crate, however
// the crate cannot fetch the proper local timezone without pulling in a bunch of
@@ -322,7 +325,7 @@ pub fn start_log_daemon() {
let file = unsafe { File::from_raw_fd(arg as RawFd) };
logfile_write_loop(file).ok();
// If any error occurs, shut down the logd pipe
*MAGISK_LOGD_FD.lock().unwrap() = None;
*MAGISK_LOGD_FD.lock() = None;
0
}
@@ -331,7 +334,7 @@ pub fn start_log_daemon() {
chown(path.as_utf8_cstr(), Some(Uid::from(0)), Some(Gid::from(0)))?;
let read = path.open(OFlag::O_RDWR | OFlag::O_CLOEXEC)?;
let write = path.open(OFlag::O_WRONLY | OFlag::O_CLOEXEC)?;
*MAGISK_LOGD_FD.lock().unwrap() = Some(Arc::new(write));
*MAGISK_LOGD_FD.lock() = Some(Arc::new(write));
unsafe {
new_daemon_thread(logfile_writer_thread, read.into_raw_fd() as usize);
}

View File

@@ -889,7 +889,7 @@ impl MagiskD {
// Handle zygisk
if self.zygisk_enabled.load(Ordering::Acquire) {
let mut zygisk = self.zygisk.lock().unwrap();
let mut zygisk = self.zygisk.lock();
zygisk.set_prop();
inject_zygisk_bins(&zygisk.lib_name, &mut system);
}

View File

@@ -10,6 +10,7 @@ use nix::mount::MsFlags;
use nix::sys::stat::{Mode, SFlag, mknod};
use num_traits::AsPrimitive;
use std::cmp::Ordering::{Greater, Less};
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
pub fn setup_preinit_dir() {
@@ -203,9 +204,8 @@ pub fn find_preinit_device() -> String {
}
Path::new(&info.source)
.file_name()
.unwrap()
.to_str()
.unwrap()
.and_then(OsStr::to_str)
.unwrap_or_default()
.to_string()
}

View File

@@ -441,7 +441,7 @@ impl MagiskD {
}
pub fn preserve_stub_apk(&self) {
let mut info = self.manager_info.lock().unwrap();
let mut info = self.manager_info.lock();
let apk = cstr::buf::default()
.join_path(get_magisk_tmp())
@@ -458,19 +458,19 @@ impl MagiskD {
}
pub fn get_manager_uid(&self, user: i32) -> i32 {
let mut info = self.manager_info.lock().unwrap();
let mut info = self.manager_info.lock();
let (uid, _) = info.get_manager(self, user, false);
uid
}
pub fn get_manager(&self, user: i32, install: bool) -> (i32, String) {
let mut info = self.manager_info.lock().unwrap();
let mut info = self.manager_info.lock();
let (uid, pkg) = info.get_manager(self, user, install);
(uid, pkg.to_string())
}
pub fn ensure_manager(&self) {
let mut info = self.manager_info.lock().unwrap();
let mut info = self.manager_info.lock();
let _ = info.get_manager(self, 0, true);
}

View File

@@ -44,7 +44,9 @@ impl Extra<'_> {
IntList(list) => {
cmd.args(["--es", self.key]);
let mut tmp = String::new();
list.iter().for_each(|i| write!(&mut tmp, "{i},").unwrap());
list.iter().for_each(|i| {
write!(&mut tmp, "{i},").ok();
});
tmp.pop();
cmd.arg(&tmp);
}
@@ -67,7 +69,9 @@ impl Extra<'_> {
IntList(list) => {
tmp = format!("{}:s:", self.key);
if !list.is_empty() {
list.iter().for_each(|i| write!(&mut tmp, "{i},").unwrap());
list.iter().for_each(|i| {
write!(&mut tmp, "{i},").ok();
});
tmp.pop();
}
}
@@ -202,8 +206,11 @@ impl SuAppContext<'_> {
let mut pfd = [PollFd::new(fd.as_fd(), PollFlags::POLLIN)];
// Wait for data input for at most 70 seconds
nix::poll::poll(&mut pfd, PollTimeout::try_from(70 * 1000).unwrap())
.check_os_err("poll", None, None)?;
nix::poll::poll(
&mut pfd,
PollTimeout::try_from(70 * 1000).unwrap_or(PollTimeout::NONE),
)
.check_os_err("poll", None, None)?;
fd
};

View File

@@ -7,11 +7,12 @@ use crate::socket::IpcRead;
use base::{LoggedResult, ResultExt, WriteExt, debug, error, exit_on_error, libc, warn};
use std::os::fd::IntoRawFd;
use std::os::unix::net::{UCred, UnixStream};
use std::sync::{Arc, Mutex};
use std::sync::Arc;
use std::time::{Duration, Instant};
#[allow(unused_imports)]
use std::os::fd::AsRawFd;
use std::sync::nonpoison::Mutex;
const DEFAULT_SHELL: &str = "/system/bin/sh";
@@ -132,7 +133,7 @@ impl MagiskD {
let info = self.get_su_info(cred.uid as i32);
{
let mut access = info.access.lock().unwrap();
let mut access = info.access.lock();
// Talk to su manager
let mut app = SuAppContext {
@@ -203,7 +204,7 @@ impl MagiskD {
}
let cached = self.cached_su_info.load();
if cached.uid == uid && cached.access.lock().unwrap().is_fresh() {
if cached.uid == uid && cached.access.lock().is_fresh() {
return cached;
}

View File

@@ -99,7 +99,6 @@ fn pump_tty_impl(ptmx: File, pump_stdin: bool) -> LoggedResult<()> {
let mut signal_fd: Option<SignalFd> = None;
let raw_ptmx = ptmx.as_raw_fd();
let mut raw_sig = -1;
let mut poll_fds = Vec::with_capacity(3);
poll_fds.push(PollFd::new(ptmx.as_fd(), PollFlags::POLLIN));
@@ -111,12 +110,14 @@ fn pump_tty_impl(ptmx: File, pump_stdin: bool) -> LoggedResult<()> {
.check_os_err("pthread_sigmask", None, None)?;
let sig = SignalFd::with_flags(&set, SfdFlags::SFD_CLOEXEC)
.into_os_result("signalfd", None, None)?;
raw_sig = sig.as_raw_fd();
signal_fd = Some(sig);
poll_fds.push(PollFd::new(
signal_fd.as_ref().unwrap().as_fd(),
PollFlags::POLLIN,
));
unsafe {
// SAFETY: signal_fd is always Some
poll_fds.push(PollFd::new(
signal_fd.as_ref().unwrap_unchecked().as_fd(),
PollFlags::POLLIN,
));
}
// We also need to pump stdin to ptmx
poll_fds.push(PollFd::new(
@@ -142,9 +143,11 @@ fn pump_tty_impl(ptmx: File, pump_stdin: bool) -> LoggedResult<()> {
pump_via_splice(FileOrStd::StdIn.as_file(), &ptmx, &pipe_fd)?;
} else if raw_fd == raw_ptmx {
pump_via_splice(&ptmx, FileOrStd::StdOut.as_file(), &pipe_fd)?;
} else if raw_fd == raw_sig {
} else if let Some(sig) = &signal_fd
&& raw_fd == sig.as_raw_fd()
{
sync_winsize(raw_ptmx);
signal_fd.as_ref().unwrap().read_signal()?;
sig.read_signal()?;
}
} else if pfd
.revents()

View File

@@ -1,7 +1,8 @@
use base::{ResultExt, new_daemon_thread};
use nix::sys::signal::SigSet;
use nix::unistd::{getpid, gettid};
use std::sync::{Condvar, LazyLock, Mutex, WaitTimeoutResult};
use std::sync::nonpoison::{Condvar, Mutex};
use std::sync::{LazyLock, WaitTimeoutResult};
use std::time::Duration;
static THREAD_POOL: LazyLock<ThreadPool> = LazyLock::new(ThreadPool::default);
@@ -33,16 +34,16 @@ impl ThreadPool {
let task: Option<Box<dyn FnOnce() + Send>>;
{
let mut info = self.info.lock().unwrap();
let mut info = self.info.lock();
info.idle_threads += 1;
if info.task.is_none() {
if is_core_pool {
// Core pool never closes, wait forever.
info = self.task_is_some.wait(info).unwrap();
info = self.task_is_some.wait(info);
} else {
let dur = Duration::from_secs(THREAD_IDLE_MAX_SEC);
let result: WaitTimeoutResult;
(info, result) = self.task_is_some.wait_timeout(info, dur).unwrap();
(info, result) = self.task_is_some.wait_timeout(info, dur);
if result.timed_out() {
// Terminate thread after timeout
info.idle_threads -= 1;
@@ -72,10 +73,10 @@ impl ThreadPool {
0
}
let mut info = self.info.lock().unwrap();
let mut info = self.info.lock();
while info.task.is_some() {
// Wait until task is none
info = self.task_is_none.wait(info).unwrap();
info = self.task_is_none.wait(info);
}
info.task = Some(Box::new(f));
if info.idle_threads == 0 {

View File

@@ -87,25 +87,23 @@ impl ZygiskState {
}
}
let socket = if let Some(fd) = socket {
fd
if let Some(fd) = socket {
fd.send_fds(&[client.as_raw_fd()])?;
} else {
// Create a new socket pair and fork zygiskd process
let (local, remote) = UnixStream::pair()?;
let (mut local, remote) = UnixStream::pair()?;
if fork_dont_care() == 0 {
exec_zygiskd(is_64_bit, remote);
}
*socket = Some(local);
let local = socket.as_mut().unwrap();
if let Some(module_fds) = daemon.get_module_fds(is_64_bit) {
local.send_fds(&module_fds)?;
}
if local.read_decodable::<i32>()? != 0 {
return log_err!();
}
local
};
socket.send_fds(&[client.as_raw_fd()])?;
local.send_fds(&[client.as_raw_fd()])?;
*socket = Some(local);
}
Ok(())
}
@@ -168,7 +166,6 @@ impl MagiskD {
ZygiskRequest::ConnectCompanion => self
.zygisk
.lock()
.unwrap()
.connect_zygiskd(client, self)
.log_with_msg(|w| w.write_str("zygiskd startup error"))?,
ZygiskRequest::GetModDir => self.get_mod_dir(client)?,
@@ -222,9 +219,12 @@ impl MagiskD {
let failed_ids: Vec<i32> = client.read_decodable()?;
if let Some(module_list) = self.module_list.get() {
for id in failed_ids {
let Some(module) = module_list.get(id as usize) else {
continue;
};
let path = cstr::buf::default()
.join_path(MODULEROOT)
.join_path(&module_list[id as usize].name)
.join_path(&module.name)
.join_path("zygisk");
// Create the unloaded marker file
if let Ok(dir) = Directory::open(&path) {
@@ -240,7 +240,13 @@ impl MagiskD {
fn get_mod_dir(&self, mut client: UnixStream) -> LoggedResult<()> {
let id: i32 = client.read_decodable()?;
let module = &self.module_list.get().unwrap()[id as usize];
let Some(module) = self
.module_list
.get()
.and_then(|list| list.get(id as usize))
else {
return Ok(());
};
let dir = cstr::buf::default()
.join_path(MODULEROOT)
.join_path(&module.name);

View File

@@ -7,6 +7,9 @@ edition.workspace = true
crate-type = ["staticlib"]
path = "lib.rs"
[lints]
workspace = true
[build-dependencies]
cxx-gen = { workspace = true }

View File

@@ -7,12 +7,15 @@ edition.workspace = true
crate-type = ["staticlib", "rlib"]
path = "lib.rs"
[build-dependencies]
cxx-gen = { workspace = true }
[features]
no-main = []
[lints]
workspace = true
[build-dependencies]
cxx-gen = { workspace = true }
[dependencies]
base = { workspace = true }
cxx = { workspace = true }