Cleanup persist props code

native/src/base/misc.rs

@@ -112,7 +112,25 @@ pub enum StrErr {
     NullPointerError,
 }
 
+pub trait StringExt {
+    fn nul_terminate(&mut self) -> &mut [u8];
+}
+
+impl StringExt for String {
+    fn nul_terminate(&mut self) -> &mut [u8] {
+        self.reserve(1);
+        // SAFETY: the string is reserved to have enough capacity to fit in the null byte
+        // SAFETY: the null byte is explicitly added outside of the string's length
+        unsafe {
+            let buf = slice::from_raw_parts_mut(self.as_mut_ptr(), self.len() + 1);
+            *buf.get_unchecked_mut(self.len()) = b'\0';
+            buf
+        }
+    }
+}
+
 // The better CStr: UTF-8 validated + null terminated buffer
+#[derive(PartialEq)]
 pub struct Utf8CStr {
     inner: [u8],
 }
@@ -129,16 +147,9 @@ impl Utf8CStr {
     }
 
     pub fn from_string(s: &mut String) -> &Utf8CStr {
-        if s.capacity() == s.len() {
-            s.reserve(1);
-        }
-        // SAFETY: the string is reserved to have enough capacity to fit in the null byte
-        // SAFETY: the null byte is explicitly added outside of the string's length
-        unsafe {
-            let buf = slice::from_raw_parts_mut(s.as_mut_ptr(), s.len() + 1);
-            *buf.get_unchecked_mut(s.len()) = b'\0';
-            Self::from_bytes_unchecked(buf)
-        }
+        let buf = s.nul_terminate();
+        // SAFETY: the null byte is explicitly added to the buffer
+        unsafe { Self::from_bytes_unchecked(buf) }
     }
 
     #[inline]

native/src/core/lib.rs

@@ -5,9 +5,7 @@ use base::Utf8CStr;
 use cert::read_certificate;
 use daemon::{daemon_entry, find_apk_path, get_magiskd, zygisk_entry, MagiskD};
 use logging::{android_logging, magisk_logging, zygisk_logging};
-use resetprop::persist::{
-    persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop,
-};
+use resetprop::{persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop};
 
 mod cert;
 #[path = "../include/consts.rs"]
@@ -19,12 +17,15 @@ mod resetprop;
 #[cxx::bridge]
 pub mod ffi {
     extern "C++" {
-        pub type prop_cb;
         include!("resetprop/resetprop.hpp");
+
+        #[cxx_name = "prop_cb"]
+        type PropCb;
+        unsafe fn get_prop_rs(name: *const c_char, persist: bool) -> String;
+        unsafe fn prop_cb_exec(cb: Pin<&mut PropCb>, name: *const c_char, value: *const c_char);
+
         include!("../base/files.hpp");
-        pub unsafe fn get_prop_rs(name: *const c_char, persist: bool) -> String;
-        pub unsafe fn prop_cb_exec(cb: *mut prop_cb, name: *const c_char, value: *const c_char);
-        pub unsafe fn clone_attr(src: *const c_char, dst: *const c_char);
+        unsafe fn clone_attr(src: *const c_char, dst: *const c_char);
     }
 
     extern "Rust" {
@@ -34,8 +35,8 @@ pub mod ffi {
         fn zygisk_logging();
         fn find_apk_path(pkg: &[u8], data: &mut [u8]) -> usize;
         fn read_certificate(fd: i32, version: i32) -> Vec<u8>;
-        unsafe fn persist_get_prop(name: *const c_char, prop_cb: *mut prop_cb);
-        unsafe fn persist_get_props(prop_cb: *mut prop_cb);
+        unsafe fn persist_get_prop(name: *const c_char, prop_cb: Pin<&mut PropCb>);
+        unsafe fn persist_get_props(prop_cb: Pin<&mut PropCb>);
         unsafe fn persist_delete_prop(name: *const c_char) -> bool;
         unsafe fn persist_set_prop(name: *const c_char, value: *const c_char) -> bool;
     }

native/src/core/resetprop.rs

@@ -0,0 +1,4 @@
+pub use persist::{persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop};
+
+mod persist;
+mod proto;

native/src/core/resetprop/mod.rs

@@ -1,2 +0,0 @@
-pub mod persist;
-pub mod proto;

native/src/core/resetprop/persist.rs

@@ -1,10 +1,5 @@
-use crate::ffi::{clone_attr, prop_cb, prop_cb_exec};
-use crate::resetprop::proto::persistent_properties::{
-    mod_PersistentProperties::PersistentPropertyRecord, PersistentProperties,
-};
-use base::{cstr, debug, libc::mkstemp, Directory, LoggedResult, MappedFile, Utf8CStr, WalkResult};
 use core::ffi::c_char;
-use quick_protobuf::{BytesReader, MessageRead, MessageWrite, Writer};
+use std::pin::Pin;
 use std::{
     fs::{read_to_string, remove_file, rename, File},
     io::{BufWriter, Write},
@@ -12,6 +7,18 @@ use std::{
     path::{Path, PathBuf},
 };
 
+use quick_protobuf::{BytesReader, MessageRead, MessageWrite, Writer};
+
+use base::{
+    cstr, debug, libc::mkstemp, Directory, LoggedResult, MappedFile, StringExt, Utf8CStr,
+    WalkResult,
+};
+
+use crate::ffi::{clone_attr, prop_cb_exec, PropCb};
+use crate::resetprop::proto::persistent_properties::{
+    mod_PersistentProperties::PersistentPropertyRecord, PersistentProperties,
+};
+
 macro_rules! PERSIST_PROP_DIR {
     () => {
         "/data/property"
@@ -24,12 +31,8 @@ macro_rules! PERSIST_PROP {
     };
 }
 
-struct PropCb {
-    cb: *mut prop_cb,
-}
-
 struct MatchNameCb<'a> {
-    cb: PropCb,
+    cb: Pin<&'a mut PropCb>,
     name: &'a Utf8CStr,
 }
 
@@ -43,17 +46,15 @@ trait PropCbExec {
     fn exec(&mut self, name: &Utf8CStr, value: &Utf8CStr);
 }
 
-impl PropCbExec for PropCb {
+impl PropCbExec for Pin<&mut PropCb> {
     fn exec(&mut self, name: &Utf8CStr, value: &Utf8CStr) {
-        if !self.cb.is_null() {
-            unsafe { prop_cb_exec(self.cb, name.as_ptr(), value.as_ptr()) }
-        }
+        unsafe { prop_cb_exec(self.as_mut(), name.as_ptr(), value.as_ptr()) }
     }
 }
 
 impl PropCbExec for MatchNameCb<'_> {
     fn exec(&mut self, name: &Utf8CStr, value: &Utf8CStr) {
-        if name.as_bytes() == self.name.as_bytes() {
+        if name == self.name {
             self.cb.exec(name, value);
             debug!("resetprop: found prop [{}] = [{}]", name, value);
         }
@@ -108,10 +109,10 @@ fn file_get_prop(name: &Utf8CStr) -> LoggedResult<String> {
 
 fn pb_write_props(props: &PersistentProperties) -> LoggedResult<()> {
     let mut tmp = String::from(concat!(PERSIST_PROP!(), ".XXXXXX"));
-    let tmp = Utf8CStr::from_string(&mut tmp);
+    tmp.nul_terminate();
     {
         let f = unsafe {
-            let fd = mkstemp(tmp.as_ptr() as *mut c_char);
+            let fd = mkstemp(tmp.as_mut_ptr().cast());
             if fd < 0 {
                 return Err(Default::default());
             }
@@ -121,7 +122,7 @@ fn pb_write_props(props: &PersistentProperties) -> LoggedResult<()> {
         props.write_message(&mut Writer::new(BufWriter::new(f)))?;
     }
     unsafe {
-        clone_attr(cstr!(PERSIST_PROP!()).as_ptr(), tmp.as_ptr());
+        clone_attr(cstr!(PERSIST_PROP!()).as_ptr(), tmp.as_ptr().cast());
     }
     rename(tmp, PERSIST_PROP!())?;
     Ok(())
@@ -151,82 +152,73 @@ fn file_set_prop(name: &Utf8CStr, value: Option<&Utf8CStr>) -> LoggedResult<()>
     Ok(())
 }
 
-fn do_persist_get_prop(name: &Utf8CStr, mut prop_cb: PropCb) -> LoggedResult<()> {
-    if check_pb() {
-        pb_get_prop(&mut MatchNameCb { cb: prop_cb, name })
-    } else {
-        let mut value = file_get_prop(name)?;
-        prop_cb.exec(name, Utf8CStr::from_string(&mut value));
-        debug!("resetprop: found prop [{}] = [{}]", name, value);
-        Ok(())
-    }
-}
-
-fn do_persist_get_props(mut prop_cb: PropCb) -> LoggedResult<()> {
-    if check_pb() {
-        pb_get_prop(&mut prop_cb)
-    } else {
-        let mut dir = Directory::open(cstr!(PERSIST_PROP_DIR!()))?;
-        dir.for_all_file(|f| {
-            if let Ok(name) = Utf8CStr::from_bytes(f.d_name().to_bytes()) {
-                if let Ok(mut value) = file_get_prop(name) {
-                    prop_cb.exec(name, Utf8CStr::from_string(&mut value));
-                }
-            }
-            Ok(WalkResult::Continue)
-        })?;
-        Ok(())
-    }
-}
-
-fn do_persist_delete_prop(name: &Utf8CStr) -> LoggedResult<()> {
-    if check_pb() {
-        let mut pp = PersistentProperties { properties: vec![] };
-        pb_get_prop(&mut PropCollectCb {
-            props: &mut pp,
-            replace_name: Some(name),
-            replace_value: None,
-        })?;
-        pb_write_props(&pp)
-    } else {
-        file_set_prop(name, None)
-    }
-}
-
-fn do_persist_set_prop(name: &Utf8CStr, value: &Utf8CStr) -> LoggedResult<()> {
-    if check_pb() {
-        let mut pp = PersistentProperties { properties: vec![] };
-        pb_get_prop(&mut PropCollectCb {
-            props: &mut pp,
-            replace_name: Some(name),
-            replace_value: Some(value),
-        })?;
-        pb_write_props(&pp)
-    } else {
-        file_set_prop(name, Some(value))
-    }
-}
-
-pub unsafe fn persist_get_prop(name: *const c_char, prop_cb: *mut prop_cb) {
-    unsafe fn inner(name: *const c_char, prop_cb: *mut prop_cb) -> LoggedResult<()> {
-        do_persist_get_prop(Utf8CStr::from_ptr(name)?, PropCb { cb: prop_cb })
+pub unsafe fn persist_get_prop(name: *const c_char, prop_cb: Pin<&mut PropCb>) {
+    unsafe fn inner(name: *const c_char, mut prop_cb: Pin<&mut PropCb>) -> LoggedResult<()> {
+        let name = Utf8CStr::from_ptr(name)?;
+        if check_pb() {
+            pb_get_prop(&mut MatchNameCb { cb: prop_cb, name })
+        } else {
+            let mut value = file_get_prop(name)?;
+            prop_cb.exec(name, Utf8CStr::from_string(&mut value));
+            debug!("resetprop: found prop [{}] = [{}]", name, value);
+            Ok(())
+        }
     }
     inner(name, prop_cb).ok();
 }
 
-pub unsafe fn persist_get_props(prop_cb: *mut prop_cb) {
-    do_persist_get_props(PropCb { cb: prop_cb }).ok();
+pub unsafe fn persist_get_props(prop_cb: Pin<&mut PropCb>) {
+    fn inner(mut prop_cb: Pin<&mut PropCb>) -> LoggedResult<()> {
+        if check_pb() {
+            pb_get_prop(&mut prop_cb)
+        } else {
+            let mut dir = Directory::open(cstr!(PERSIST_PROP_DIR!()))?;
+            dir.for_all_file(|f| {
+                if let Ok(name) = Utf8CStr::from_bytes(f.d_name().to_bytes()) {
+                    if let Ok(mut value) = file_get_prop(name) {
+                        prop_cb.exec(name, Utf8CStr::from_string(&mut value));
+                    }
+                }
+                Ok(WalkResult::Continue)
+            })?;
+            Ok(())
+        }
+    }
+    inner(prop_cb).ok();
 }
 
 pub unsafe fn persist_delete_prop(name: *const c_char) -> bool {
     unsafe fn inner(name: *const c_char) -> LoggedResult<()> {
-        do_persist_delete_prop(Utf8CStr::from_ptr(name)?)
+        let name = Utf8CStr::from_ptr(name)?;
+        if check_pb() {
+            let mut pp = PersistentProperties { properties: vec![] };
+            pb_get_prop(&mut PropCollectCb {
+                props: &mut pp,
+                replace_name: Some(name),
+                replace_value: None,
+            })?;
+            pb_write_props(&pp)
+        } else {
+            file_set_prop(name, None)
+        }
     }
     inner(name).is_ok()
 }
 pub unsafe fn persist_set_prop(name: *const c_char, value: *const c_char) -> bool {
     unsafe fn inner(name: *const c_char, value: *const c_char) -> LoggedResult<()> {
-        do_persist_set_prop(Utf8CStr::from_ptr(name)?, Utf8CStr::from_ptr(value)?)
+        let name = Utf8CStr::from_ptr(name)?;
+        let value = Utf8CStr::from_ptr(value)?;
+        if check_pb() {
+            let mut pp = PersistentProperties { properties: vec![] };
+            pb_get_prop(&mut PropCollectCb {
+                props: &mut pp,
+                replace_name: Some(name),
+                replace_value: Some(value),
+            })?;
+            pb_write_props(&pp)
+        } else {
+            file_set_prop(name, Some(value))
+        }
     }
     inner(name, value).is_ok()
 }

native/src/core/resetprop/resetprop.cpp

@@ -6,6 +6,7 @@
 #include <base.hpp>
 
 #include "resetprop.hpp"
+#include "../core-rs.hpp"
 
 using namespace std;
 
@@ -195,7 +196,7 @@ static StringType get_prop(const char *name, PropFlags flags) {
     }
 
     if (cb.val.empty() && flags.isPersist() && str_starts(name, "persist."))
-        persist_get_prop(name, &cb);
+        persist_get_prop(name, cb);
     if (cb.val.empty())
         LOGD("resetprop: prop [%s] does not exist\n", name);
 
@@ -208,7 +209,7 @@ static void print_props(PropFlags flags) {
     if (!flags.isPersistOnly())
         system_property_foreach(read_prop_with_cb, &collector);
     if (flags.isPersist())
-        persist_get_props(&collector);
+        persist_get_props(collector);
     for (auto &[key, val] : list) {
         const char *v = flags.isContext() ?
                 (__system_property_get_context(key.data()) ?: "") :
@@ -258,7 +259,7 @@ struct Initialize {
 };
 
 static void InitOnce() {
-    struct Initialize init;
+    static struct Initialize init;
 }
 
 #define consume_next(val)    \
@@ -269,6 +270,7 @@ stop_parse = true;           \
 int resetprop_main(int argc, char *argv[]) {
     PropFlags flags;
     char *argv0 = argv[0];
+    set_log_level_state(LogLevel::Debug, false);
 
     const char *prop_file = nullptr;
     const char *prop_to_rm = nullptr;

native/src/core/resetprop/resetprop.hpp

@@ -29,11 +29,6 @@ int delete_prop(const char *name, bool persist = false);
 int set_prop(const char *name, const char *value, bool skip_svc = false);
 void load_prop_file(const char *filename, bool skip_svc = false);
 
-inline void prop_cb_exec(prop_cb *cb, const char *name, const char *value) {
-    cb->exec(name, value);
+static inline void prop_cb_exec(prop_cb &cb, const char *name, const char *value) {
+    cb.exec(name, value);
 }
-
-void persist_get_prop(const char *name, prop_cb *prop_cb) noexcept;
-void persist_get_props(prop_cb *prop_cb) noexcept;
-bool persist_delete_prop(const char *name) noexcept;
-bool persist_set_prop(const char *name, const char *value) noexcept;