From 8a0156e022f5363220acd6d84840107a4b3c5c87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 11 Oct 2023 15:17:33 +0200 Subject: [PATCH] Remove StrBuf --- esp-wifi/src/ble/btdm.rs | 10 +- esp-wifi/src/ble/npl.rs | 16 +- .../common_adapter/common_adapter_esp32c2.rs | 7 +- .../common_adapter/common_adapter_esp32c3.rs | 7 +- .../common_adapter/common_adapter_esp32c6.rs | 7 +- esp-wifi/src/common_adapter/mod.rs | 14 +- esp-wifi/src/compat/common.rs | 142 +++++++++--------- esp-wifi/src/wifi/os_adapter.rs | 28 ++-- 8 files changed, 110 insertions(+), 121 deletions(-) diff --git a/esp-wifi/src/ble/btdm.rs b/esp-wifi/src/ble/btdm.rs index 4433a310..217ac3b3 100644 --- a/esp-wifi/src/ble/btdm.rs +++ b/esp-wifi/src/ble/btdm.rs @@ -7,7 +7,7 @@ use crate::ble::HciOutCollector; use crate::ble::HCI_OUT_COLLECTOR; use crate::{ binary::include::*, - compat::{common::StrBuf, queue::SimpleQueue, work_queue::queue_work}, + compat::{common::str_from_c, queue::SimpleQueue, work_queue::queue_work}, memory_fence::memory_fence, timer::yield_task, }; @@ -288,12 +288,12 @@ unsafe extern "C" fn task_create( handle: *mut crate::binary::c_types::c_void, core_id: u32, ) -> i32 { - let n = StrBuf::from(name); + let n = str_from_c(name); trace!( "task_create {:?} {:?} {} {} {:?} {} {:?} {}", func, name, - n.as_str_ref(), + n, stack_depth, param, prio, @@ -479,8 +479,8 @@ pub(crate) fn ble_init() { } let version = btdm_controller_get_compile_version(); - let version_str = StrBuf::from(version); - debug!("BT controller compile version {}", version_str.as_str_ref()); + let version_str = str_from_c(version); + debug!("BT controller compile version {}", version_str); ble_os_adapter_chip_specific::bt_periph_module_enable(); diff --git a/esp-wifi/src/ble/npl.rs b/esp-wifi/src/ble/npl.rs index 01f5b2e5..d02b670f 100644 --- a/esp-wifi/src/ble/npl.rs +++ b/esp-wifi/src/ble/npl.rs @@ -6,7 +6,7 @@ use super::*; use crate::binary::c_types::c_void; use crate::binary::include::*; use crate::compat; -use crate::compat::common::StrBuf; +use crate::compat::common::str_from_c; use crate::compat::queue::SimpleQueue; use crate::timer::yield_task; @@ -372,11 +372,11 @@ unsafe extern "C" fn task_create( task_handle: *const crate::binary::c_types::c_void, core_id: u32, ) -> i32 { - let name_str = StrBuf::from(name as *const u8); + let name_str = str_from_c(name as *const u8); trace!( "task_create {:?} {} {} {:?} {} {:?} {}", task_func, - name_str.as_str_ref(), + name_str, stack_depth, param, prio, @@ -409,14 +409,8 @@ unsafe extern "C" fn osi_assert( param1: u32, param2: u32, ) { - let name_str = StrBuf::from(fn_name as *const u8); - panic!( - "ASSERT {}:{} {} {}", - name_str.as_str_ref(), - ln, - param1, - param2 - ); + let name_str = str_from_c(fn_name as *const u8); + panic!("ASSERT {}:{} {} {}", name_str, ln, param1, param2); } unsafe extern "C" fn esp_intr_free(_ret_handle: *mut *mut crate::binary::c_types::c_void) -> i32 { diff --git a/esp-wifi/src/common_adapter/common_adapter_esp32c2.rs b/esp-wifi/src/common_adapter/common_adapter_esp32c2.rs index 38300a73..95f77c2f 100644 --- a/esp-wifi/src/common_adapter/common_adapter_esp32c2.rs +++ b/esp-wifi/src/common_adapter/common_adapter_esp32c2.rs @@ -1,7 +1,7 @@ use super::phy_init_data::PHY_INIT_DATA_DEFAULT; use crate::binary::include::*; use crate::common_adapter::RADIO_CLOCKS; -use crate::compat::common::StrBuf; +use crate::compat::common::str_from_c; use crate::hal::system::RadioClockController; use crate::hal::system::RadioPeripherals; use atomic_polyfill::AtomicU32; @@ -36,10 +36,7 @@ pub(crate) unsafe fn phy_enable() { [0u8; core::mem::size_of::()]; let phy_version = get_phy_version_str(); - trace!( - "phy_version {}", - StrBuf::from(phy_version as *const u8).as_str_ref() - ); + trace!("phy_version {}", str_from_c(phy_version as *const u8)); let init_data = &PHY_INIT_DATA_DEFAULT; diff --git a/esp-wifi/src/common_adapter/common_adapter_esp32c3.rs b/esp-wifi/src/common_adapter/common_adapter_esp32c3.rs index ab99883b..2c8cb7e7 100644 --- a/esp-wifi/src/common_adapter/common_adapter_esp32c3.rs +++ b/esp-wifi/src/common_adapter/common_adapter_esp32c3.rs @@ -1,7 +1,7 @@ use super::phy_init_data::PHY_INIT_DATA_DEFAULT; use crate::binary::include::*; use crate::common_adapter::RADIO_CLOCKS; -use crate::compat::common::StrBuf; +use crate::compat::common::str_from_c; use crate::hal::system::RadioClockController; use crate::hal::system::RadioPeripherals; use atomic_polyfill::AtomicU32; @@ -71,10 +71,7 @@ pub(crate) unsafe fn phy_enable() { [0u8; core::mem::size_of::()]; let phy_version = get_phy_version_str(); - trace!( - "phy_version {}", - StrBuf::from(phy_version as *const u8).as_str_ref() - ); + trace!("phy_version {}", str_from_c(phy_version as *const u8)); let init_data = &PHY_INIT_DATA_DEFAULT; diff --git a/esp-wifi/src/common_adapter/common_adapter_esp32c6.rs b/esp-wifi/src/common_adapter/common_adapter_esp32c6.rs index 34f968d1..18878436 100644 --- a/esp-wifi/src/common_adapter/common_adapter_esp32c6.rs +++ b/esp-wifi/src/common_adapter/common_adapter_esp32c6.rs @@ -1,7 +1,7 @@ use super::phy_init_data::PHY_INIT_DATA_DEFAULT; use crate::binary::include::*; use crate::common_adapter::RADIO_CLOCKS; -use crate::compat::common::StrBuf; +use crate::compat::common::str_from_c; use crate::hal::system::RadioClockController; use crate::hal::system::RadioPeripherals; use atomic_polyfill::AtomicU32; @@ -36,10 +36,7 @@ pub(crate) unsafe fn phy_enable() { [0u8; core::mem::size_of::()]; let phy_version = get_phy_version_str(); - trace!( - "phy_version {}", - StrBuf::from(phy_version as *const u8).as_str_ref() - ); + trace!("phy_version {}", str_from_c(phy_version as *const u8)); let init_data = &PHY_INIT_DATA_DEFAULT; diff --git a/esp-wifi/src/common_adapter/mod.rs b/esp-wifi/src/common_adapter/mod.rs index 7a5b19aa..0be51934 100644 --- a/esp-wifi/src/common_adapter/mod.rs +++ b/esp-wifi/src/common_adapter/mod.rs @@ -318,19 +318,19 @@ pub(crate) unsafe extern "C" fn semphr_give_from_isr(sem: *const (), hptw: *cons // other functions #[no_mangle] pub unsafe extern "C" fn puts(s: *const u8) { - let cstr = StrBuf::from(s); - trace!("{}", cstr.as_str_ref()); + let cstr = str_from_c(s); + trace!("{}", cstr); } #[no_mangle] pub unsafe extern "C" fn sprintf(dst: *mut u8, format: *const u8, args: ...) -> i32 { - let str = StrBuf::from(format); - trace!("sprintf {}", str.as_str_ref()); + let str = str_from_c(format); + trace!("sprintf format: {}", str); - let len = crate::compat::common::vsnprintf(dst, 511, format, args); + let len = crate::compat::common::vsnprintf(dst, 512, format, args); - let s = StrBuf::from(dst); - trace!("sprintf {}", s.as_str_ref()); + let s = str_from_c(dst); + trace!("sprintf result: {}", s); len } diff --git a/esp-wifi/src/compat/common.rs b/esp-wifi/src/compat/common.rs index 87fe78ed..8d67963a 100644 --- a/esp-wifi/src/compat/common.rs +++ b/esp-wifi/src/compat/common.rs @@ -34,102 +34,109 @@ static mut MUTEX_IDX_CURRENT: usize = 0; static mut FAKE_WIFI_QUEUE: &Option> = unsafe { &REAL_WIFI_QUEUE }; static mut REAL_WIFI_QUEUE: Option> = None; // first there is a ptr to the real queue - driver checks it's not null -pub struct StrBuf { - buffer: [u8; 512], +pub struct StrWriter { + dst: *mut u8, + capacity: usize, len: usize, } -impl StrBuf { - pub fn new() -> StrBuf { - StrBuf { - buffer: [0u8; 512], +impl StrWriter { + pub fn new(dst: *mut u8, capacity: usize) -> Self { + Self { + dst, + capacity, len: 0, } } - pub unsafe fn from(c_str: *const u8) -> StrBuf { - let mut res = StrBuf { - buffer: [0u8; 512], - len: 0, - }; - - let mut idx: usize = 0; - while *(c_str.offset(idx as isize)) != 0 { - res.buffer[idx] = *(c_str.offset(idx as isize)); - idx += 1; - } + pub fn len(&self) -> usize { + self.len + } - res.len = idx; - res + fn space(&self) -> usize { + self.capacity - self.len } - pub unsafe fn append_from(&mut self, c_str: *const u8) { - let mut src_idx: usize = 0; - let mut idx: usize = self.len; - while *(c_str.offset(src_idx as isize)) != 0 { - self.buffer[idx] = *(c_str.offset(src_idx as isize)); - idx += 1; - src_idx += 1; + fn write(&mut self, byte: u8) { + unsafe { + self.dst.write(b); + self.dst = self.dst.add(1); } + } - self.len = idx; + pub fn append_char(&mut self, c: char) { + let mut buf = [0u8; 4]; + let char = c.encode_utf8(&mut buf); + self.append(char); } pub fn append(&mut self, s: &str) { - let mut idx: usize = self.len; - s.chars().for_each(|c| { - self.buffer[idx] = c as u8; - idx += 1; - }); - self.len = idx; - } + // Write as many bytes as possible. We're writing a c string which means we don't have + // to deal with utf8 character boundaries, so this should be fine. + let len = s.len().min(self.space()); + let fitting = &s.as_bytes()[..len]; + for byte in fitting { + self.write(*byte); + } - pub fn append_char(&mut self, c: char) { - let mut idx: usize = self.len; - self.buffer[idx] = c as u8; - idx += 1; - self.len = idx; + // vsnprintf's semantics: it counts unwritten bytes, too + self.len += s.len(); } - pub unsafe fn as_str_ref(&self) -> &str { - core::str::from_utf8_unchecked(&self.buffer[..self.len]) + pub fn append_byte(&mut self, b: u8) { + if self.space() >= 1 { + self.write(b); + } + + // vsnprintf's semantics: it counts unwritten bytes, too + self.len += 1; } } -impl Write for StrBuf { +impl Write for StrWriter { fn write_str(&mut self, s: &str) -> Result<(), core::fmt::Error> { self.append(s); Ok(()) } } -pub unsafe extern "C" fn syslog(_priority: u32, format: *const u8, mut args: VaListImpl) { - #[cfg(all(feature = "wifi-logs", target_arch = "riscv32"))] - { - let mut buf = [0u8; 512]; - vsnprintf(&mut buf as *mut u8, 511, format, args); - let res_str = StrBuf::from(&buf as *const u8); - info!("{}", res_str.as_str_ref()); +pub unsafe fn str_from_c<'a>(s: *const u8) -> &'a str { + let mut len = 0; + while *(s.add(len)) != 0 { + len += 1; } - #[cfg(all(feature = "wifi-logs", not(target_arch = "riscv32")))] + let slice = core::slice::from_raw_parts(s, len); + core::str::from_utf8_unchecked(slice) +} + +pub unsafe extern "C" fn syslog(_priority: u32, format: *const u8, mut args: VaListImpl) { + #[cfg(feature = "wifi-logs")] { - let res_str = StrBuf::from(format); - info!("{}", res_str.as_str_ref()); + #[cfg(target_arch = "riscv32")] + { + let mut buf = [0u8; 512]; + vsnprintf(&mut buf as *mut u8, 512, format, args); + let res_str = str_from_c(&buf as *const u8); + info!("{}", res_str); + } + #[cfg(not(target_arch = "riscv32"))] + { + let res_str = str_from_c(format); + info!("{}", res_str); + } } } +/// Returns the number of character that would have been written if the buffer was big enough. pub(crate) unsafe fn vsnprintf( dst: *mut u8, - _n: u32, + capacity: u32, format: *const u8, mut args: VaListImpl, ) -> i32 { - let fmt_str_ptr = format; + let mut res_str = StrWriter::new(dst, capacity as usize - 1); - let mut res_str = StrBuf::new(); - - let strbuf = StrBuf::from(fmt_str_ptr); - let s = strbuf.as_str_ref(); + let s = str_from_c(format); let mut format_char = ' '; let mut is_long = false; @@ -159,6 +166,7 @@ pub(crate) unsafe fn vsnprintf( // have to format an arg match format_char { 'd' => { + // FIXME: This is sus - both branches have the same impl if is_long { let v = args.arg::(); write!(res_str, "{}", v).ok(); @@ -190,14 +198,14 @@ pub(crate) unsafe fn vsnprintf( 's' => { let v = args.arg::() as *const u8; - let vbuf = StrBuf::from(v); - write!(res_str, "{}", vbuf.as_str_ref()).ok(); + let vbuf = str_from_c(v); + res_str.append(vbuf); } 'c' => { let v = args.arg::(); if v != 0 { - write!(res_str, "{}", v as char).ok(); + res_str.append_byte(v); } } @@ -211,14 +219,12 @@ pub(crate) unsafe fn vsnprintf( is_long = false; } } - let mut idx = 0; - res_str.as_str_ref().chars().for_each(|c| { - *(dst.offset(idx)) = c as u8; - idx += 1; - }); - *(dst.offset(idx)) = 0; - idx as i32 + let chars_written = res_str.len(); + let terminating_at = chars_written.min(capacity as usize - 1); + dst.add(terminating_at).write(0); + + chars_written as i32 } #[no_mangle] diff --git a/esp-wifi/src/wifi/os_adapter.rs b/esp-wifi/src/wifi/os_adapter.rs index 6a2b6c23..11fcdd5b 100644 --- a/esp-wifi/src/wifi/os_adapter.rs +++ b/esp-wifi/src/wifi/os_adapter.rs @@ -17,7 +17,7 @@ use crate::{ compat::{ common::{ create_recursive_mutex, create_wifi_queue, lock_mutex, receive_queued, send_queued, - thread_sem_get, unlock_mutex, StrBuf, + str_from_c, thread_sem_get, unlock_mutex, }, malloc::calloc, work_queue::queue_work, @@ -697,7 +697,7 @@ pub unsafe extern "C" fn task_create_pinned_to_core( ) -> i32 { trace!("task_create_pinned_to_core task_func {:?} name {} stack_depth {} param {:?} prio {}, task_handle {:?} core_id {}", task_func, - StrBuf::from(name as *const u8).as_str_ref(), + str_from_c(name as *const u8), stack_depth, param, prio, @@ -1538,21 +1538,19 @@ pub unsafe extern "C" fn log_writev( _format: *const crate::binary::c_types::c_char, _args: va_list, ) { - #[cfg(not(feature = "wifi-logs"))] - return; - - #[cfg(target_arch = "xtensa")] - #[allow(unreachable_code)] + #[cfg(feature = "wifi-logs")] { - let s = StrBuf::from(_format as *const u8); - info!("{}", s.as_str_ref()); - } + #[cfg(target_arch = "xtensa")] + { + let s = str_from_c(_format as *const u8); + info!("{}", s); + } - #[cfg(target_arch = "riscv32")] - #[allow(unreachable_code)] - { - let _args = core::mem::transmute(_args); - syslog(_level, _format as *const u8, _args); + #[cfg(target_arch = "riscv32")] + { + let _args = core::mem::transmute(_args); + syslog(_level, _format as *const u8, _args); + } } }