From 6804d1ea5bd768bd069c719a4c6e1a72e88b5adb Mon Sep 17 00:00:00 2001 From: Petr Portnov Date: Sun, 3 Dec 2023 21:51:34 +0300 Subject: [PATCH] refactor(GH-92): rollback all changes to `c_string` in favor of #120 --- crates/Cargo.toml | 1 - crates/flipperzero/examples/storage.rs | 2 +- crates/flipperzero/src/dialogs/mod.rs | 40 +++++++++++++++++---- crates/flipperzero/src/furi/log/mod.rs | 49 +++++++++----------------- crates/sys/Cargo.toml | 1 - crates/sys/src/lib.rs | 12 ++----- 6 files changed, 55 insertions(+), 50 deletions(-) diff --git a/crates/Cargo.toml b/crates/Cargo.toml index 4a56d54a..9458feb6 100644 --- a/crates/Cargo.toml +++ b/crates/Cargo.toml @@ -23,7 +23,6 @@ flipperzero-sys = { path = "sys", version = "0.11.0" } flipperzero-rt = { path = "rt", version = "0.11.0" } flipperzero-alloc = { path = "alloc", version = "0.11.0" } flipperzero-test = { path = "test", version = "0.11.0" } -cstr = "0.2.11" ufmt = "0.2.0" document-features = "0.2.0" diff --git a/crates/flipperzero/examples/storage.rs b/crates/flipperzero/examples/storage.rs index 3fe91641..9eaf88e2 100644 --- a/crates/flipperzero/examples/storage.rs +++ b/crates/flipperzero/examples/storage.rs @@ -43,7 +43,7 @@ fn main(_args: *mut u8) -> i32 { // Next, we'll open a file browser dialog and let the user select the file. let mut dialogs_app = DialogsApp::open(); - let file_browser_options = DialogFileBrowserOptions::new(c"*").set_hide_ext(false); + let file_browser_options = DialogFileBrowserOptions::new().set_hide_ext(false); let mut start_path = FuriString::from(path); let result_path = dialogs_app.show_file_browser(Some(&mut start_path), Some(&file_browser_options)); diff --git a/crates/flipperzero/src/dialogs/mod.rs b/crates/flipperzero/src/dialogs/mod.rs index 15e5b305..4d242a0c 100644 --- a/crates/flipperzero/src/dialogs/mod.rs +++ b/crates/flipperzero/src/dialogs/mod.rs @@ -201,9 +201,28 @@ impl DialogMessageButton { } } +impl<'a> Default for DialogFileBrowserOptions<'a> { + fn default() -> Self { + Self::new() + } +} + impl<'a> DialogFileBrowserOptions<'a> { + pub fn new() -> Self { + // SAFETY: the string is a valid UTF-8 + unsafe { Self::with_extension(c"*") } + } + /// Creates a new dialog file browser options and initializes to default values. /// + /// # Safety + /// + /// `extension` should be a valid UTF-8 string + /// + /// # Compatibility + /// + /// This function's signature may change in the future to make it safe. + /// /// # Examples /// /// Basic usage: @@ -221,14 +240,14 @@ impl<'a> DialogFileBrowserOptions<'a> { /// ``` /// # use core::ffi::CStr; /// # use flipperzero::dialogs::DialogFileBrowserOptions; - /// # use flipperzero_sys::{cstr, DialogsFileBrowserOptions}; /// // has `'static` lifetime - /// const EXTENSION: &CStr = cstr!("*"); + /// const EXTENSION: &CStr = c"txt"; /// // has "local" lifetime, aka `'a` /// let base_path_bytes = [b'/', b'r', b'o', b'o', b't']; /// let base_path = CStr::from_bytes_with_nul(&base_path_bytes).unwrap(); /// // the most appropriate lifetime `'a` is used - /// let mut options = DialogFileBrowserOptions::new(EXTENSION) + /// // SAFETY: `EXTENSION` is a valid UTF-8 string + /// let mut options = unsafe { DialogFileBrowserOptions::new(EXTENSION) } /// .set_base_path(base_path); /// ``` /// @@ -239,14 +258,15 @@ impl<'a> DialogFileBrowserOptions<'a> { /// # use flipperzero::dialogs::DialogFileBrowserOptions; /// # use flipperzero_sys::{cstr, DialogsFileBrowserOptions}; /// const EXTENSION: &CStr = cstr!("*"); - /// let mut options = DialogFileBrowserOptions::new(EXTENSION); + /// // SAFETY: `EXTENSION` is a valid UTF-8 string + /// let mut options = unsafe { DialogFileBrowserOptions::new(EXTENSION) }; /// { /// let base_path_bytes = [b'/', b'r', b'o', b'o', b't']; /// let base_path = CStr::from_bytes_with_nul(&base_path_bytes).unwrap(); /// options = options.set_base_path(base_path); /// } /// ``` - pub fn new(extension: &'a CStr) -> Self { + pub unsafe fn with_extension(extension: &'a CStr) -> Self { let mut options = MaybeUninit::::uninit(); let uninit_options = options.as_mut_ptr(); let extension = extension.as_ptr(); @@ -267,7 +287,15 @@ impl<'a> DialogFileBrowserOptions<'a> { } /// Set file extension to be offered for selection. - pub fn set_extension(mut self, extension: &'a CStr) -> Self { + /// + /// # Safety + /// + /// `extension` should be a valid UTF-8 string + /// + /// # Compatibility + /// + /// This function's signature may change in the future to make it safe. + pub unsafe fn set_extension(mut self, extension: &'a CStr) -> Self { self.data.extension = extension.as_ptr(); self } diff --git a/crates/flipperzero/src/furi/log/mod.rs b/crates/flipperzero/src/furi/log/mod.rs index 7c85dc93..daef0982 100644 --- a/crates/flipperzero/src/furi/log/mod.rs +++ b/crates/flipperzero/src/furi/log/mod.rs @@ -22,39 +22,24 @@ pub use metadata::{Level, LevelFilter}; /// ``` #[macro_export] macro_rules! log { - (target: $target:literal, $lvl:expr, $msg:literal $(, $arg:expr)*) => ({ - $crate::log!(@unsafe { - target: $crate::__macro_support::__sys::c_string!($target), - $lvl, $msg $(, $arg)* - }) - }); - - ($lvl:expr, $msg:literal $(, $arg:expr)*) => ( - $crate::log!(@unsafe { - target: concat!(module_path!(), "\0").as_ptr() as *const core::ffi::c_char, - $lvl, $msg $(, $arg)* - }) - ); - - (@unsafe { - target: $target_cstr_ptr:expr, - $lvl:expr, $msg:literal $(, $arg:expr)* - }) => ({ + (target: $target:expr, $lvl:expr, $msg:expr $(, $arg:expr)*) => ({ if $lvl <= $crate::furi::log::LevelFilter::current() { let mut buf = $crate::__macro_support::FuriString::new(); $crate::__macro_support::ufmt::uwrite!(&mut buf, $msg $(, $arg)*) .expect("can append to FuriString"); - // SAFETY: `$target_cstr_ptr` should be a valid pointer - // and `buf` has just been created safely unsafe { $crate::__macro_support::__sys::furi_log_print_format( $crate::__macro_support::__level_to_furi($lvl), - $target_cstr_ptr, + $crate::__macro_support::__sys::c_string!($target), buf.as_c_str().as_ptr(), ); } } - }) + }); + + ($lvl:expr, $msg:expr $(, $arg:expr)*) => ( + $crate::log!(target: module_path!(), $lvl, $msg $(, $arg)*) + ); } /// Logs a message at the error level. @@ -74,11 +59,11 @@ macro_rules! log { /// ``` #[macro_export] macro_rules! error { - (target: $target:expr, $msg:literal $(, $arg:expr)*) => ( + (target: $target:expr, $msg:expr $(, $arg:expr)*) => ( $crate::log!(target: $target, $crate::furi::log::Level::ERROR, $msg $(, $arg)*) ); - ($msg:literal $(, $arg:expr)*) => ( + ($msg:expr $(, $arg:expr)*) => ( $crate::log!($crate::furi::log::Level::ERROR, $msg $(, $arg)*) ); } @@ -99,11 +84,11 @@ macro_rules! error { /// ``` #[macro_export] macro_rules! warn { - (target: $target:expr, $msg:literal $(, $arg:expr)*) => ( + (target: $target:expr, $msg:expr $(, $arg:expr)*) => ( $crate::log!(target: $target, $crate::furi::log::Level::WARN, $msg $(, $arg)*) ); - ($msg:literal $(, $arg:expr)*) => ( + ($msg:expr $(, $arg:expr)*) => ( $crate::log!($crate::furi::log::Level::WARN, $msg $(, $arg)*) ); } @@ -124,11 +109,11 @@ macro_rules! warn { /// ``` #[macro_export] macro_rules! info { - (target: $target:expr, $msg:literal $(, $arg:expr)*) => ( + (target: $target:expr, $msg:expr $(, $arg:expr)*) => ( $crate::log!(target: $target, $crate::furi::log::Level::INFO, $msg $(, $arg)*) ); - ($msg:literal $(, $arg:expr)*) => ( + ($msg:expr $(, $arg:expr)*) => ( $crate::log!($crate::furi::log::Level::INFO, $msg $(, $arg)*) ); } @@ -149,11 +134,11 @@ macro_rules! info { /// ``` #[macro_export] macro_rules! debug { - (target: $target:expr, $msg:literal $(, $arg:expr)*) => ( + (target: $target:expr, $msg:expr $(, $arg:expr)*) => ( $crate::log!(target: $target, $crate::furi::log::Level::DEBUG, $msg $(, $arg)*) ); - ($msg:literal $(, $arg:expr)*) => ( + ($msg:expr $(, $arg:expr)*) => ( $crate::log!($crate::furi::log::Level::DEBUG, $msg $(, $arg)*) ); } @@ -174,11 +159,11 @@ macro_rules! debug { /// ``` #[macro_export] macro_rules! trace { - (target: $target:expr, $msg:literal $(, $arg:expr)*) => ( + (target: $target:expr, $msg:expr $(, $arg:expr)*) => ( $crate::log!(target: $target, $crate::furi::log::Level::TRACE, $msg $(, $arg)*) ); - ($msg:literal $(, $arg:expr)*) => ( + ($msg:expr $(, $arg:expr)*) => ( $crate::log!($crate::furi::log::Level::TRACE, $msg $(, $arg)*) ); } diff --git a/crates/sys/Cargo.toml b/crates/sys/Cargo.toml index f5914a5d..93a56f9e 100644 --- a/crates/sys/Cargo.toml +++ b/crates/sys/Cargo.toml @@ -22,5 +22,4 @@ bench = false test = false [dependencies] -cstr.workspace = true ufmt.workspace = true diff --git a/crates/sys/src/lib.rs b/crates/sys/src/lib.rs index fd848828..d61e607d 100644 --- a/crates/sys/src/lib.rs +++ b/crates/sys/src/lib.rs @@ -29,18 +29,12 @@ mod inlines; )] mod bindings; -/// Create a static C string of type [`&'static CStr`][`core::ffi::CStr`]. +/// Create a static C string. /// Will automatically add a NUL terminator. -pub use cstr::cstr; - -/// Create a static C string of type [`*const c_char`][core::ffi::c_char] -/// referring to a `'static` string. -/// Will automatically add a NUL terminator. -// TODO: don't produce intermediate `CStr` whose `length` part we don't use #[macro_export] macro_rules! c_string { ($str:expr $(,)?) => {{ - $crate::cstr!($str).as_ptr() + concat!($str, "\0").as_ptr() as *const core::ffi::c_char }}; } @@ -53,7 +47,7 @@ macro_rules! crash { let msg = $crate::c_string!($msg); core::arch::asm!("", in("r12") msg, options(nomem, nostack)); - $crate::__furi_crash(); + $crate::__furi_crash_implementation(); core::hint::unreachable_unchecked(); } };