From 060b14f068ea49a1aaad65d5dce84e301d4afc0c Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Sat, 26 Feb 2022 14:37:26 +0100 Subject: [PATCH 1/9] Fixed duplicate services and constistency in naming --- ctru-rs/examples/buttons.rs | 2 +- ctru-rs/examples/file-explorer.rs | 4 +- ctru-rs/examples/futures-basic.rs | 2 +- ctru-rs/examples/futures-tokio.rs | 2 +- ctru-rs/examples/gfx-wide-mode.rs | 2 +- ctru-rs/examples/graphics-bitmap.rs | 2 +- ctru-rs/examples/hashmaps.rs | 2 +- ctru-rs/examples/hello-both-screens.rs | 2 +- ctru-rs/examples/hello-world.rs | 2 +- ctru-rs/examples/network-sockets.rs | 2 +- ctru-rs/examples/romfs.rs | 4 +- ctru-rs/examples/thread-basic.rs | 2 +- ctru-rs/examples/thread-locals.rs | 2 +- ctru-rs/examples/time-rtc.rs | 2 +- ctru-rs/src/error.rs | 3 ++ ctru-rs/src/gfx.rs | 58 +++++++++++++++++--------- ctru-rs/src/lib.rs | 29 ++++++++++--- ctru-rs/src/romfs.rs | 38 +++++++++++++---- ctru-rs/src/services/apt.rs | 50 +++++++++++++++------- ctru-rs/src/services/fs.rs | 42 +++++++++++++++---- ctru-rs/src/services/hid.rs | 39 +++++++++++++---- ctru-rs/src/services/ps.rs | 2 - ctru-rs/src/services/soc.rs | 51 +++++++++++++++------- ctru-rs/src/services/sslc.rs | 49 ++++++++++++++++------ ctru-rs/src/srv.rs | 38 +++++++++++++---- ctru-rs/src/test_runner.rs | 7 +--- 26 files changed, 320 insertions(+), 118 deletions(-) diff --git a/ctru-rs/examples/buttons.rs b/ctru-rs/examples/buttons.rs index 152ede6..ca4ba3a 100644 --- a/ctru-rs/examples/buttons.rs +++ b/ctru-rs/examples/buttons.rs @@ -8,7 +8,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); let console = Console::init(gfx.top_screen.borrow_mut()); println!("Hi there! Try pressing a button"); diff --git a/ctru-rs/examples/file-explorer.rs b/ctru-rs/examples/file-explorer.rs index 5a638ee..0451989 100644 --- a/ctru-rs/examples/file-explorer.rs +++ b/ctru-rs/examples/file-explorer.rs @@ -14,10 +14,10 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); #[cfg(all(feature = "romfs", romfs_exists))] - let _romfs = ctru::romfs::RomFS::new().unwrap(); + let _romfs = ctru::romfs::RomFS::init().unwrap(); FileExplorer::init(&apt, &hid, &gfx).run(); } diff --git a/ctru-rs/examples/futures-basic.rs b/ctru-rs/examples/futures-basic.rs index c48964b..dd78795 100644 --- a/ctru-rs/examples/futures-basic.rs +++ b/ctru-rs/examples/futures-basic.rs @@ -13,7 +13,7 @@ use futures::StreamExt; fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/futures-tokio.rs b/ctru-rs/examples/futures-tokio.rs index 7eb3d03..a478603 100644 --- a/ctru-rs/examples/futures-tokio.rs +++ b/ctru-rs/examples/futures-tokio.rs @@ -6,7 +6,7 @@ use std::time::Duration; fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/gfx-wide-mode.rs b/ctru-rs/examples/gfx-wide-mode.rs index fcf3fa9..1d39a2f 100644 --- a/ctru-rs/examples/gfx-wide-mode.rs +++ b/ctru-rs/examples/gfx-wide-mode.rs @@ -7,7 +7,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); let mut console = Console::init(gfx.top_screen.borrow_mut()); println!("Press A to enable/disable wide screen mode."); diff --git a/ctru-rs/examples/graphics-bitmap.rs b/ctru-rs/examples/graphics-bitmap.rs index fc65c9d..822d03d 100644 --- a/ctru-rs/examples/graphics-bitmap.rs +++ b/ctru-rs/examples/graphics-bitmap.rs @@ -19,7 +19,7 @@ static IMAGE: &[u8] = include_bytes!("assets/ferris.rgb"); fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/hashmaps.rs b/ctru-rs/examples/hashmaps.rs index f5377f1..8dc26e2 100644 --- a/ctru-rs/examples/hashmaps.rs +++ b/ctru-rs/examples/hashmaps.rs @@ -12,7 +12,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); let _console = Console::init(gfx.top_screen.borrow_mut()); let mut map = std::collections::HashMap::new(); diff --git a/ctru-rs/examples/hello-both-screens.rs b/ctru-rs/examples/hello-both-screens.rs index 67113a3..5a96187 100644 --- a/ctru-rs/examples/hello-both-screens.rs +++ b/ctru-rs/examples/hello-both-screens.rs @@ -8,7 +8,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); // Start a console on the top screen let top_screen = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/hello-world.rs b/ctru-rs/examples/hello-world.rs index 930549f..b620025 100644 --- a/ctru-rs/examples/hello-world.rs +++ b/ctru-rs/examples/hello-world.rs @@ -7,7 +7,7 @@ use std::io::BufWriter; fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().expect("Couldn't obtain GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/network-sockets.rs b/ctru-rs/examples/network-sockets.rs index 84978e9..c459619 100644 --- a/ctru-rs/examples/network-sockets.rs +++ b/ctru-rs/examples/network-sockets.rs @@ -10,7 +10,7 @@ use std::time::Duration; fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); let _console = Console::init(gfx.top_screen.borrow_mut()); let hid = Hid::init().unwrap(); let apt = Apt::init().unwrap(); diff --git a/ctru-rs/examples/romfs.rs b/ctru-rs/examples/romfs.rs index ac8b443..6e7ece9 100644 --- a/ctru-rs/examples/romfs.rs +++ b/ctru-rs/examples/romfs.rs @@ -5,7 +5,7 @@ use ctru::services::hid::{Hid, KeyPad}; fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); @@ -15,7 +15,7 @@ fn main() { // This never fails as `ctru-rs` examples inherit all of the `ctru` features, // but it might if a normal user application wasn't setup correctly if #[cfg(all(feature = "romfs", romfs_exists))] { - let _romfs = ctru::romfs::RomFS::new().unwrap(); + let _romfs = ctru::romfs::RomFS::init().unwrap(); let f = std::fs::read_to_string("romfs:/test-file.txt").unwrap(); println!("Contents of test-file.txt: \n{f}\n"); diff --git a/ctru-rs/examples/thread-basic.rs b/ctru-rs/examples/thread-basic.rs index 7657718..5da5560 100644 --- a/ctru-rs/examples/thread-basic.rs +++ b/ctru-rs/examples/thread-basic.rs @@ -11,7 +11,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); let _console = Console::init(gfx.top_screen.borrow_mut()); let prio = thread::current().priority(); diff --git a/ctru-rs/examples/thread-locals.rs b/ctru-rs/examples/thread-locals.rs index 937855a..a6618a1 100644 --- a/ctru-rs/examples/thread-locals.rs +++ b/ctru-rs/examples/thread-locals.rs @@ -10,7 +10,7 @@ std::thread_local! { fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); gfx.top_screen.borrow_mut().set_wide_mode(true); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); diff --git a/ctru-rs/examples/time-rtc.rs b/ctru-rs/examples/time-rtc.rs index 23096ed..696a7f0 100644 --- a/ctru-rs/examples/time-rtc.rs +++ b/ctru-rs/examples/time-rtc.rs @@ -6,7 +6,7 @@ use ctru::services::hid::{Hid, KeyPad}; fn main() { ctru::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); diff --git a/ctru-rs/src/error.rs b/ctru-rs/src/error.rs index 53c3fbe..790866c 100644 --- a/ctru-rs/src/error.rs +++ b/ctru-rs/src/error.rs @@ -9,6 +9,7 @@ pub type Result = ::std::result::Result; #[non_exhaustive] pub enum Error { Os(ctru_sys::Result), + ServiceAlreadyActive(&'static str), } impl From for Error { @@ -28,6 +29,7 @@ impl fmt::Debug for Error { .field("summary", &R_SUMMARY(err)) .field("level", &R_LEVEL(err)) .finish(), + Error::ServiceAlreadyActive(service) => write!(f, "Service {service} already active"), } } } @@ -39,6 +41,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { Error::Os(err) => write!(f, "libctru result code: 0x{:08X}", err), + Error::ServiceAlreadyActive(service) => write!(f, "Service {service} already active"), } } } diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index c761549..b38419f 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -1,10 +1,11 @@ //! LCD screens manipulation helper use std::cell::RefCell; -use std::default::Default; use std::marker::PhantomData; use std::ops::Drop; +use std::sync::atomic::{AtomicBool, Ordering}; +use crate::error::{Error, Result}; use crate::services::gspgpu::{self, FramebufferFormat}; /// Trait implemented by TopScreen and BottomScreen for common methods @@ -73,25 +74,39 @@ pub struct Gfx { pub bottom_screen: RefCell, } +static GFX_ACTIVE: AtomicBool = AtomicBool::new(false); + impl Gfx { /// Initialize the Gfx module with the chosen framebuffer formats for the top and bottom /// screens /// - /// Use `Gfx::default()` instead of this function to initialize the module with default parameters - pub fn new( + /// Use `Gfx::init_default()` instead of this function to initialize the module with default parameters + pub fn init( top_fb_fmt: FramebufferFormat, bottom_fb_fmt: FramebufferFormat, use_vram_buffers: bool, - ) -> Self { - unsafe { - ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), use_vram_buffers); - } - Gfx { - top_screen: RefCell::new(TopScreen), - bottom_screen: RefCell::new(BottomScreen), + ) -> Result { + match GFX_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => { + unsafe { + ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), use_vram_buffers); + } + + Ok(Gfx { + top_screen: RefCell::new(TopScreen), + bottom_screen: RefCell::new(BottomScreen), + }) + } + Err(_) => Err(Error::ServiceAlreadyActive("Gfx")), } } + /// Creates a new Gfx instance with default init values + /// It's the same as calling: `Gfx::init(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false) + pub fn init_default() -> Result { + Gfx::init(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false) + } + /// Flushes the current framebuffers pub fn flush_buffers(&self) { unsafe { ctru_sys::gfxFlushBuffers() }; @@ -197,18 +212,21 @@ impl From for ctru_sys::gfx3dSide_t { } } -impl Default for Gfx { - fn default() -> Self { - unsafe { ctru_sys::gfxInitDefault() }; - Gfx { - top_screen: RefCell::new(TopScreen), - bottom_screen: RefCell::new(BottomScreen), - } - } -} - impl Drop for Gfx { fn drop(&mut self) { unsafe { ctru_sys::gfxExit() }; + + GFX_ACTIVE.store(false, Ordering::Release); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn gfx_duplicate() { + // We don't need to build a `Gfx` because the test runner has one already + assert!(Gfx::init_default().is_err()); } } diff --git a/ctru-rs/src/lib.rs b/ctru-rs/src/lib.rs index 89756d4..85b9c63 100644 --- a/ctru-rs/src/lib.rs +++ b/ctru-rs/src/lib.rs @@ -38,12 +38,31 @@ pub fn init() { // Only for panics in the main thread if main_thread == thread::current().id() && console::Console::exists() { println!("\nPress SELECT to exit the software"); - let hid = services::hid::Hid::init().unwrap(); - loop { - hid.scan_input(); - if hid.keys_down().contains(services::hid::KeyPad::KEY_SELECT) { - break; + // The use of unsafe functions here is basically obligatory. + // To have memory safety when using the `Hid` struct, we must not make more + // than one available at the same time, so no drop/service ownership issues arise. + // The problem here is that the `panic_hook` is run _before_ the app cleanup, + // so an `Hid` stuct may still be alive and thus make the `panic_hook` panic. + // If that were to happen, the system would have to reboot to properly close the app. + // + // Using `hidInit` is safe when another instance is open, and we can do safe operations afterwards. + // The only (probably) unsafe part of this system is the `hidExit`, since in a multithreaded + // environment some other threads may still be doing operations on the service + // before the cleanup, though the time window would be almost nonexistent, and it would only + // really be a problem in preemptive threads. + // + // TL;DR : This code is bad. + unsafe { + ctru_sys::hidInit(); + + loop { + ctru_sys::hidScanInput(); + let keys = services::hid::KeyPad::from_bits_truncate(ctru_sys::hidKeysDown()); + if keys.contains(services::hid::KeyPad::KEY_SELECT) { + ctru_sys::hidExit(); + break; + } } } } diff --git a/ctru-rs/src/romfs.rs b/ctru-rs/src/romfs.rs index 12190b9..19b0990 100644 --- a/ctru-rs/src/romfs.rs +++ b/ctru-rs/src/romfs.rs @@ -11,19 +11,29 @@ //! ``` use std::ffi::CStr; +use std::sync::atomic::{AtomicBool, Ordering}; + +use crate::error::Error; #[non_exhaustive] pub struct RomFS; +static ROMFS_ACTIVE: AtomicBool = AtomicBool::new(false); + impl RomFS { - pub fn new() -> crate::Result { - let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); - let result = unsafe { ctru_sys::romfsMountSelf(mount_name.as_ptr()) }; + pub fn init() -> crate::Result { + match ROMFS_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => { + let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); + let result = unsafe { ctru_sys::romfsMountSelf(mount_name.as_ptr()) }; - if result < 0 { - Err(result.into()) - } else { - Ok(Self) + if result < 0 { + Err(result.into()) + } else { + Ok(Self) + } + } + Err(_) => Err(Error::ServiceAlreadyActive("RomFS")), } } } @@ -32,5 +42,19 @@ impl Drop for RomFS { fn drop(&mut self) { let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); unsafe { ctru_sys::romfsUnmount(mount_name.as_ptr()) }; + + ROMFS_ACTIVE.store(false, Ordering::Release); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn romfs_duplicate() { + let _romfs = RomFS::init().unwrap(); + + assert!(RomFS::init().is_err()); } } diff --git a/ctru-rs/src/services/apt.rs b/ctru-rs/src/services/apt.rs index 4662a4e..2e425ff 100644 --- a/ctru-rs/src/services/apt.rs +++ b/ctru-rs/src/services/apt.rs @@ -1,14 +1,24 @@ +use std::sync::atomic::{AtomicBool, Ordering}; + +use crate::error::Error; + +#[non_exhaustive] pub struct Apt(()); +static APT_ACTIVE: AtomicBool = AtomicBool::new(false); + impl Apt { - pub fn init() -> crate::Result { - unsafe { - let r = ctru_sys::aptInit(); - if r < 0 { - Err(r.into()) - } else { - Ok(Apt(())) + pub fn init() -> crate::Result { + match APT_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => { + let r = unsafe { ctru_sys::aptInit() }; + if r < 0 { + Err(r.into()) + } else { + Ok(Self(())) + } } + Err(_) => Err(Error::ServiceAlreadyActive("Apt")), } } @@ -17,13 +27,11 @@ impl Apt { } pub fn set_app_cpu_time_limit(&self, percent: u32) -> crate::Result<()> { - unsafe { - let r = ctru_sys::APT_SetAppCpuTimeLimit(percent); - if r < 0 { - Err(r.into()) - } else { - Ok(()) - } + let r = unsafe { ctru_sys::APT_SetAppCpuTimeLimit(percent) }; + if r < 0 { + Err(r.into()) + } else { + Ok(()) } } } @@ -31,5 +39,19 @@ impl Apt { impl Drop for Apt { fn drop(&mut self) { unsafe { ctru_sys::aptExit() }; + + APT_ACTIVE.store(false, Ordering::Release); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn gfx_duplicate() { + // We don't need to build a `Apt` because the test runner has one already + assert!(Apt::init().is_err()); + } +} + diff --git a/ctru-rs/src/services/fs.rs b/ctru-rs/src/services/fs.rs index 9f3334a..cc9bbe7 100644 --- a/ctru-rs/src/services/fs.rs +++ b/ctru-rs/src/services/fs.rs @@ -13,9 +13,12 @@ use std::mem; use std::path::{Path, PathBuf}; use std::ptr; use std::slice; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use widestring::{WideCStr, WideCString}; +use crate::error::Error; + bitflags! { #[derive(Default)] struct FsOpen: u32 { @@ -82,8 +85,11 @@ pub enum ArchiveID { /// until an instance of this struct is created. /// /// The service exits when all instances of this struct go out of scope. +#[non_exhaustive] pub struct Fs(()); +static FS_ACTIVE: AtomicBool = AtomicBool::new(false); + /// Handle to an open filesystem archive. /// /// Archives are automatically closed when they go out of scope. @@ -96,6 +102,7 @@ pub struct Fs(()); /// let fs = Fs::init().unwrap(); /// let sdmc_archive = fs.sdmc().unwrap(); /// ``` +#[non_exhaustive] pub struct Archive { id: ArchiveID, handle: u64, @@ -164,6 +171,7 @@ pub struct Archive { /// # Ok(()) /// # } /// ``` +#[non_exhaustive] pub struct File { handle: u32, offset: u64, @@ -175,6 +183,7 @@ pub struct File { /// represents known metadata about a file. /// /// [`metadata`]: fn.metadata.html +#[non_exhaustive] pub struct Metadata { attributes: u32, size: u64, @@ -257,6 +266,7 @@ pub struct OpenOptions { /// /// This Result will return Err if there's some sort of intermittent IO error /// during iteration. +#[non_exhaustive] pub struct ReadDir<'a> { handle: Dir, root: Arc, @@ -297,14 +307,17 @@ impl Fs { /// ctrulib services are reference counted, so this function may be called /// as many times as desired and the service will not exit until all /// instances of Fs drop out of scope. - pub fn init() -> crate::Result { - unsafe { - let r = ctru_sys::fsInit(); - if r < 0 { - Err(r.into()) - } else { - Ok(Fs(())) + pub fn init() -> crate::Result { + match FS_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => { + let r = unsafe { ctru_sys::fsInit() }; + if r < 0 { + Err(r.into()) + } else { + Ok(Self(())) + } } + Err(_) => Err(Error::ServiceAlreadyActive("Fs")), } } @@ -990,6 +1003,8 @@ impl Drop for Fs { unsafe { ctru_sys::fsExit(); } + + FS_ACTIVE.store(false, Ordering::Release); } } @@ -1059,3 +1074,16 @@ impl From for ctru_sys::FS_ArchiveID { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fs_duplicate() { + let _fs = Fs::init().unwrap(); + + assert!(Fs::init().is_err()); + } +} + diff --git a/ctru-rs/src/services/hid.rs b/ctru-rs/src/services/hid.rs index 18790f1..6ad67fb 100644 --- a/ctru-rs/src/services/hid.rs +++ b/ctru-rs/src/services/hid.rs @@ -4,6 +4,10 @@ //! and circle pad information. It also provides information from the sound volume slider, //! the accelerometer, and the gyroscope. +use std::sync::atomic::{AtomicBool, Ordering}; + +use crate::error::Error; + bitflags::bitflags! { /// A set of flags corresponding to the button and directional pad /// inputs on the 3DS @@ -44,12 +48,17 @@ bitflags::bitflags! { /// when all instances of this struct fall out of scope. /// /// This service requires no special permissions to use. +#[non_exhaustive] pub struct Hid(()); +static HID_ACTIVE: AtomicBool = AtomicBool::new(false); + /// Represents user input to the touchscreen. +#[non_exhaustive] pub struct TouchPosition(ctru_sys::touchPosition); /// Represents the current position of the 3DS circle pad. +#[non_exhaustive] pub struct CirclePosition(ctru_sys::circlePosition); /// Initializes the HID service. @@ -60,14 +69,17 @@ pub struct CirclePosition(ctru_sys::circlePosition); /// Since this service requires no special or elevated permissions, errors are /// rare in practice. impl Hid { - pub fn init() -> crate::Result { - unsafe { - let r = ctru_sys::hidInit(); - if r < 0 { - Err(r.into()) - } else { - Ok(Hid(())) + pub fn init() -> crate::Result { + match HID_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => { + let r = unsafe { ctru_sys::hidInit() }; + if r < 0 { + Err(r.into()) + } else { + Ok(Self(())) + } } + Err(_) => Err(Error::ServiceAlreadyActive("Hid")), } } @@ -151,5 +163,18 @@ impl CirclePosition { impl Drop for Hid { fn drop(&mut self) { unsafe { ctru_sys::hidExit() }; + + HID_ACTIVE.store(false, Ordering::Release); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hid_duplicate() { + // We don't need to build a `Hid` because the test runner has one already + assert!(Hid::init().is_err()); } } diff --git a/ctru-rs/src/services/ps.rs b/ctru-rs/src/services/ps.rs index 8a419f4..3b4ef1a 100644 --- a/ctru-rs/src/services/ps.rs +++ b/ctru-rs/src/services/ps.rs @@ -63,8 +63,6 @@ pub fn generate_random_bytes(out: &mut [u8]) -> crate::Result<()> { mod tests { use std::collections::HashMap; - use super::*; - #[test] fn construct_hash_map() { let mut input = vec![ diff --git a/ctru-rs/src/services/soc.rs b/ctru-rs/src/services/soc.rs index 026bd2d..a15541a 100644 --- a/ctru-rs/src/services/soc.rs +++ b/ctru-rs/src/services/soc.rs @@ -1,21 +1,27 @@ -use ctru_sys::{socExit, socInit}; use libc::{free, memalign}; use std::net::Ipv4Addr; +use std::sync::atomic::{AtomicBool, Ordering}; + +use crate::error::Error; +use ctru_sys::{socExit, socInit}; /// Soc service. Initializing this service will enable the use of network sockets and utilities /// such as those found in `std::net`. The service will be closed when this struct is is dropped. +#[non_exhaustive] pub struct Soc { soc_mem: *mut u32, } +static SOC_ACTIVE: AtomicBool = AtomicBool::new(false); + impl Soc { /// Initialize the Soc service with a default buffer size of 0x100000 bytes /// /// # Errors /// /// This function will return an error if the `Soc` service is already initialized - pub fn init() -> crate::Result { - Soc::init_with_buffer_size(0x100000) + pub fn init() -> crate::Result { + Self::init_with_buffer_size(0x100000) } /// Initialize the Soc service with a custom buffer size in bytes. The size should be @@ -24,17 +30,20 @@ impl Soc { /// # Errors /// /// This function will return an error if the `Soc` service is already initialized - pub fn init_with_buffer_size(num_bytes: usize) -> crate::Result { - unsafe { - let soc_mem = memalign(0x1000, num_bytes) as *mut u32; - - let r = socInit(soc_mem, num_bytes as u32); - if r < 0 { - free(soc_mem as *mut _); - Err(r.into()) - } else { - Ok(Soc { soc_mem }) - } + pub fn init_with_buffer_size(num_bytes: usize) -> crate::Result { + match SOC_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => unsafe { + let soc_mem = memalign(0x1000, num_bytes) as *mut u32; + + let r = socInit(soc_mem, num_bytes as u32); + if r < 0 { + free(soc_mem as *mut _); + Err(r.into()) + } else { + Ok(Self { soc_mem }) + } + }, + Err(_) => Err(Error::ServiceAlreadyActive("Soc")), } } @@ -51,5 +60,19 @@ impl Drop for Soc { socExit(); free(self.soc_mem as *mut _); } + + SOC_ACTIVE.store(false, Ordering::Release); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn soc_duplicate() { + let _soc = Soc::init().unwrap(); + + assert!(Soc::init().is_err()); } } diff --git a/ctru-rs/src/services/sslc.rs b/ctru-rs/src/services/sslc.rs index bdd5407..0b2192e 100644 --- a/ctru-rs/src/services/sslc.rs +++ b/ctru-rs/src/services/sslc.rs @@ -1,29 +1,37 @@ // TODO: Implement remaining functions +use std::sync::atomic::{AtomicBool, Ordering}; + +use crate::error::Error; + +#[non_exhaustive] pub struct SslC(()); +static SSLC_ACTIVE: AtomicBool = AtomicBool::new(false); + impl SslC { /// Initialize sslc pub fn init() -> crate::Result { - unsafe { - let r = ctru_sys::sslcInit(0); - if r < 0 { - Err(r.into()) - } else { - Ok(SslC(())) + match SSLC_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => { + let r = unsafe { ctru_sys::sslcInit(0) }; + if r < 0 { + Err(r.into()) + } else { + Ok(Self(())) + } } + Err(_) => Err(Error::ServiceAlreadyActive("SslC")), } } /// Fill `buf` with `buf.len()` random bytes pub fn generate_random_data(&self, buf: &mut [u8]) -> crate::Result<()> { - unsafe { - let r = ctru_sys::sslcGenerateRandomData(buf.as_ptr() as _, buf.len() as u32); - if r < 0 { - Err(r.into()) - } else { - Ok(()) - } + let r = unsafe { ctru_sys::sslcGenerateRandomData(buf.as_ptr() as _, buf.len() as u32) }; + if r < 0 { + Err(r.into()) + } else { + Ok(()) } } } @@ -31,5 +39,20 @@ impl SslC { impl Drop for SslC { fn drop(&mut self) { unsafe { ctru_sys::sslcExit() }; + + SSLC_ACTIVE.store(false, Ordering::Release); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sslc_duplicate() { + let _sslc = SslC::init().unwrap(); + + assert!(SslC::init().is_err()); + } +} + diff --git a/ctru-rs/src/srv.rs b/ctru-rs/src/srv.rs index 7d2ad21..8cca5c3 100644 --- a/ctru-rs/src/srv.rs +++ b/ctru-rs/src/srv.rs @@ -1,14 +1,24 @@ +use std::sync::atomic::{AtomicBool, Ordering}; + +use crate::error::Error; + +#[non_exhaustive] pub struct Srv(()); +static SRV_ACTIVE: AtomicBool = AtomicBool::new(false); + impl Srv { - pub fn init() -> crate::Result { - unsafe { - let r = ctru_sys::srvInit(); - if r < 0 { - Err(r.into()) - } else { - Ok(Srv(())) + pub fn init() -> crate::Result { + match SRV_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + Ok(_) => { + let r = unsafe { ctru_sys::srvInit() }; + if r < 0 { + Err(r.into()) + } else { + Ok(Self(())) + } } + Err(_) => Err(Error::ServiceAlreadyActive("Srv")), } } } @@ -16,5 +26,19 @@ impl Srv { impl Drop for Srv { fn drop(&mut self) { unsafe { ctru_sys::srvExit() }; + + SRV_ACTIVE.store(false, Ordering::Release); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn srv_duplicate() { + let _srv = Srv::init().unwrap(); + + assert!(Srv::init().is_err()); } } diff --git a/ctru-rs/src/test_runner.rs b/ctru-rs/src/test_runner.rs index ff3a459..0e9ab06 100644 --- a/ctru-rs/src/test_runner.rs +++ b/ctru-rs/src/test_runner.rs @@ -17,7 +17,7 @@ use crate::services::Apt; pub(crate) fn run(tests: &[&TestDescAndFn]) { crate::init(); - let gfx = Gfx::default(); + let gfx = Gfx::init_default().unwrap(); let hid = Hid::init().unwrap(); let apt = Apt::init().unwrap(); @@ -115,9 +115,4 @@ mod link_fix { extern "C" fn sigemptyset(_arg1: *mut libc::sigset_t) -> ::libc::c_int { -1 } - - #[no_mangle] - extern "C" fn sysconf(_name: libc::c_int) -> libc::c_long { - -1 - } } From 447ef1eaa2246ead59ab1b5c6bbe16129feb4208 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Thu, 10 Mar 2022 19:16:28 +0100 Subject: [PATCH 2/9] Fixed typos, nits and small issues --- ctru-rs/examples/buttons.rs | 2 +- ctru-rs/examples/file-explorer.rs | 2 +- ctru-rs/examples/futures-basic.rs | 2 +- ctru-rs/examples/futures-tokio.rs | 2 +- ctru-rs/examples/gfx-wide-mode.rs | 2 +- ctru-rs/examples/graphics-bitmap.rs | 2 +- ctru-rs/examples/hashmaps.rs | 2 +- ctru-rs/examples/hello-both-screens.rs | 2 +- ctru-rs/examples/hello-world.rs | 2 +- ctru-rs/examples/network-sockets.rs | 2 +- ctru-rs/examples/romfs.rs | 2 +- ctru-rs/examples/software-keyboard.rs | 2 +- ctru-rs/examples/thread-basic.rs | 2 +- ctru-rs/examples/thread-locals.rs | 2 +- ctru-rs/examples/time-rtc.rs | 2 +- ctru-rs/src/error.rs | 5 ++++- ctru-rs/src/gfx.rs | 17 ++++++++++------- ctru-rs/src/lib.rs | 2 +- ctru-rs/src/romfs.rs | 7 +++++-- ctru-rs/src/services/apt.rs | 10 ++++++---- ctru-rs/src/services/fs.rs | 8 +++++--- ctru-rs/src/services/hid.rs | 7 +++++-- ctru-rs/src/services/soc.rs | 7 +++++-- ctru-rs/src/services/sslc.rs | 8 +++++--- ctru-rs/src/srv.rs | 7 +++++-- ctru-rs/src/test_runner.rs | 2 +- 26 files changed, 67 insertions(+), 43 deletions(-) diff --git a/ctru-rs/examples/buttons.rs b/ctru-rs/examples/buttons.rs index ca4ba3a..e3250c4 100644 --- a/ctru-rs/examples/buttons.rs +++ b/ctru-rs/examples/buttons.rs @@ -8,7 +8,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); let console = Console::init(gfx.top_screen.borrow_mut()); println!("Hi there! Try pressing a button"); diff --git a/ctru-rs/examples/file-explorer.rs b/ctru-rs/examples/file-explorer.rs index 0451989..181c846 100644 --- a/ctru-rs/examples/file-explorer.rs +++ b/ctru-rs/examples/file-explorer.rs @@ -14,7 +14,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); #[cfg(all(feature = "romfs", romfs_exists))] let _romfs = ctru::romfs::RomFS::init().unwrap(); diff --git a/ctru-rs/examples/futures-basic.rs b/ctru-rs/examples/futures-basic.rs index dd78795..f027372 100644 --- a/ctru-rs/examples/futures-basic.rs +++ b/ctru-rs/examples/futures-basic.rs @@ -13,7 +13,7 @@ use futures::StreamExt; fn main() { ctru::init(); - let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); + let gfx = Gfx::init().expect("Couldn't obtain GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/futures-tokio.rs b/ctru-rs/examples/futures-tokio.rs index a478603..91f821e 100644 --- a/ctru-rs/examples/futures-tokio.rs +++ b/ctru-rs/examples/futures-tokio.rs @@ -6,7 +6,7 @@ use std::time::Duration; fn main() { ctru::init(); - let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); + let gfx = Gfx::init().expect("Couldn't obtain GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/gfx-wide-mode.rs b/ctru-rs/examples/gfx-wide-mode.rs index 1d39a2f..f84f5c4 100644 --- a/ctru-rs/examples/gfx-wide-mode.rs +++ b/ctru-rs/examples/gfx-wide-mode.rs @@ -7,7 +7,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); let mut console = Console::init(gfx.top_screen.borrow_mut()); println!("Press A to enable/disable wide screen mode."); diff --git a/ctru-rs/examples/graphics-bitmap.rs b/ctru-rs/examples/graphics-bitmap.rs index 822d03d..0d82ecf 100644 --- a/ctru-rs/examples/graphics-bitmap.rs +++ b/ctru-rs/examples/graphics-bitmap.rs @@ -19,7 +19,7 @@ static IMAGE: &[u8] = include_bytes!("assets/ferris.rgb"); fn main() { ctru::init(); - let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); + let gfx = Gfx::init().expect("Couldn't obtain GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/hashmaps.rs b/ctru-rs/examples/hashmaps.rs index 8dc26e2..0b07939 100644 --- a/ctru-rs/examples/hashmaps.rs +++ b/ctru-rs/examples/hashmaps.rs @@ -12,7 +12,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); let _console = Console::init(gfx.top_screen.borrow_mut()); let mut map = std::collections::HashMap::new(); diff --git a/ctru-rs/examples/hello-both-screens.rs b/ctru-rs/examples/hello-both-screens.rs index 5a96187..483e5c3 100644 --- a/ctru-rs/examples/hello-both-screens.rs +++ b/ctru-rs/examples/hello-both-screens.rs @@ -8,7 +8,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); // Start a console on the top screen let top_screen = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/hello-world.rs b/ctru-rs/examples/hello-world.rs index b620025..2fcd7fb 100644 --- a/ctru-rs/examples/hello-world.rs +++ b/ctru-rs/examples/hello-world.rs @@ -7,7 +7,7 @@ use std::io::BufWriter; fn main() { ctru::init(); - let gfx = Gfx::init_default().expect("Couldn't obtain GFX controller"); + let gfx = Gfx::init().expect("Couldn't obtain GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/network-sockets.rs b/ctru-rs/examples/network-sockets.rs index c459619..2172d81 100644 --- a/ctru-rs/examples/network-sockets.rs +++ b/ctru-rs/examples/network-sockets.rs @@ -10,7 +10,7 @@ use std::time::Duration; fn main() { ctru::init(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); let _console = Console::init(gfx.top_screen.borrow_mut()); let hid = Hid::init().unwrap(); let apt = Apt::init().unwrap(); diff --git a/ctru-rs/examples/romfs.rs b/ctru-rs/examples/romfs.rs index 6e7ece9..dc1d133 100644 --- a/ctru-rs/examples/romfs.rs +++ b/ctru-rs/examples/romfs.rs @@ -5,7 +5,7 @@ use ctru::services::hid::{Hid, KeyPad}; fn main() { ctru::init(); - let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); + let gfx = Gfx::init().expect("Couldn't obtain GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); let _console = Console::init(gfx.top_screen.borrow_mut()); diff --git a/ctru-rs/examples/software-keyboard.rs b/ctru-rs/examples/software-keyboard.rs index 6be9e81..799d65d 100644 --- a/ctru-rs/examples/software-keyboard.rs +++ b/ctru-rs/examples/software-keyboard.rs @@ -8,7 +8,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::default(); + let gfx = Gfx::init().unwrap(); let _console = Console::init(gfx.top_screen.borrow_mut()); println!("Press A to enter some text or press Start to quit"); diff --git a/ctru-rs/examples/thread-basic.rs b/ctru-rs/examples/thread-basic.rs index 5da5560..09002a1 100644 --- a/ctru-rs/examples/thread-basic.rs +++ b/ctru-rs/examples/thread-basic.rs @@ -11,7 +11,7 @@ fn main() { ctru::init(); let apt = Apt::init().unwrap(); let hid = Hid::init().unwrap(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); let _console = Console::init(gfx.top_screen.borrow_mut()); let prio = thread::current().priority(); diff --git a/ctru-rs/examples/thread-locals.rs b/ctru-rs/examples/thread-locals.rs index a6618a1..593f90a 100644 --- a/ctru-rs/examples/thread-locals.rs +++ b/ctru-rs/examples/thread-locals.rs @@ -10,7 +10,7 @@ std::thread_local! { fn main() { ctru::init(); - let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); + let gfx = Gfx::init().expect("Couldn't obtain GFX controller"); gfx.top_screen.borrow_mut().set_wide_mode(true); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); diff --git a/ctru-rs/examples/time-rtc.rs b/ctru-rs/examples/time-rtc.rs index 696a7f0..b77615a 100644 --- a/ctru-rs/examples/time-rtc.rs +++ b/ctru-rs/examples/time-rtc.rs @@ -6,7 +6,7 @@ use ctru::services::hid::{Hid, KeyPad}; fn main() { ctru::init(); - let gfx = Gfx::init_default().expect("Couldn't obtaint GFX controller"); + let gfx = Gfx::init().expect("Couldn't obtain GFX controller"); let hid = Hid::init().expect("Couldn't obtain HID controller"); let apt = Apt::init().expect("Couldn't obtain APT controller"); diff --git a/ctru-rs/src/error.rs b/ctru-rs/src/error.rs index 790866c..2cfe55a 100644 --- a/ctru-rs/src/error.rs +++ b/ctru-rs/src/error.rs @@ -29,7 +29,10 @@ impl fmt::Debug for Error { .field("summary", &R_SUMMARY(err)) .field("level", &R_LEVEL(err)) .finish(), - Error::ServiceAlreadyActive(service) => write!(f, "Service {service} already active"), + Error::ServiceAlreadyActive(service) => f + .debug_tuple("ServiceAlreadyActive") + .field(&String::from(service)) + .finish(), } } } diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index b38419f..b03e64d 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -80,8 +80,8 @@ impl Gfx { /// Initialize the Gfx module with the chosen framebuffer formats for the top and bottom /// screens /// - /// Use `Gfx::init_default()` instead of this function to initialize the module with default parameters - pub fn init( + /// Use `Gfx::init()` instead of this function to initialize the module with default parameters + pub fn with_formats( top_fb_fmt: FramebufferFormat, bottom_fb_fmt: FramebufferFormat, use_vram_buffers: bool, @@ -102,9 +102,9 @@ impl Gfx { } /// Creates a new Gfx instance with default init values - /// It's the same as calling: `Gfx::init(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false) - pub fn init_default() -> Result { - Gfx::init(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false) + /// It's the same as calling: `Gfx::with_formats(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false) + pub fn init() -> Result { + Gfx::with_formats(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false) } /// Flushes the current framebuffers @@ -216,7 +216,7 @@ impl Drop for Gfx { fn drop(&mut self) { unsafe { ctru_sys::gfxExit() }; - GFX_ACTIVE.store(false, Ordering::Release); + GFX_ACTIVE.store(false, Ordering::SeqCst); } } @@ -227,6 +227,9 @@ mod tests { #[test] fn gfx_duplicate() { // We don't need to build a `Gfx` because the test runner has one already - assert!(Gfx::init_default().is_err()); + match Gfx::init() { + Err(Error::ServiceAlreadyActive("Gfx")) => return, + _ => panic!(), + } } } diff --git a/ctru-rs/src/lib.rs b/ctru-rs/src/lib.rs index 85b9c63..8cad7a9 100644 --- a/ctru-rs/src/lib.rs +++ b/ctru-rs/src/lib.rs @@ -45,7 +45,7 @@ pub fn init() { // The problem here is that the `panic_hook` is run _before_ the app cleanup, // so an `Hid` stuct may still be alive and thus make the `panic_hook` panic. // If that were to happen, the system would have to reboot to properly close the app. - // + // // Using `hidInit` is safe when another instance is open, and we can do safe operations afterwards. // The only (probably) unsafe part of this system is the `hidExit`, since in a multithreaded // environment some other threads may still be doing operations on the service diff --git a/ctru-rs/src/romfs.rs b/ctru-rs/src/romfs.rs index 19b0990..f09c25e 100644 --- a/ctru-rs/src/romfs.rs +++ b/ctru-rs/src/romfs.rs @@ -43,7 +43,7 @@ impl Drop for RomFS { let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); unsafe { ctru_sys::romfsUnmount(mount_name.as_ptr()) }; - ROMFS_ACTIVE.store(false, Ordering::Release); + ROMFS_ACTIVE.store(false, Ordering::SeqCst); } } @@ -55,6 +55,9 @@ mod tests { fn romfs_duplicate() { let _romfs = RomFS::init().unwrap(); - assert!(RomFS::init().is_err()); + match RomFS::init() { + Err(Error::ServiceAlreadyActive("RomFS")) => return, + _ => panic!(), + } } } diff --git a/ctru-rs/src/services/apt.rs b/ctru-rs/src/services/apt.rs index 2e425ff..85fb710 100644 --- a/ctru-rs/src/services/apt.rs +++ b/ctru-rs/src/services/apt.rs @@ -40,7 +40,7 @@ impl Drop for Apt { fn drop(&mut self) { unsafe { ctru_sys::aptExit() }; - APT_ACTIVE.store(false, Ordering::Release); + APT_ACTIVE.store(false, Ordering::SeqCst); } } @@ -49,9 +49,11 @@ mod tests { use super::*; #[test] - fn gfx_duplicate() { + fn apt_duplicate() { // We don't need to build a `Apt` because the test runner has one already - assert!(Apt::init().is_err()); + match Apt::init() { + Err(Error::ServiceAlreadyActive("Apt")) => return, + _ => panic!(), + } } } - diff --git a/ctru-rs/src/services/fs.rs b/ctru-rs/src/services/fs.rs index cc9bbe7..b4c7cfa 100644 --- a/ctru-rs/src/services/fs.rs +++ b/ctru-rs/src/services/fs.rs @@ -1004,7 +1004,7 @@ impl Drop for Fs { ctru_sys::fsExit(); } - FS_ACTIVE.store(false, Ordering::Release); + FS_ACTIVE.store(false, Ordering::SeqCst); } } @@ -1083,7 +1083,9 @@ mod tests { fn fs_duplicate() { let _fs = Fs::init().unwrap(); - assert!(Fs::init().is_err()); + match Fs::init() { + Err(Error::ServiceAlreadyActive("Fs")) => return, + _ => panic!(), + } } } - diff --git a/ctru-rs/src/services/hid.rs b/ctru-rs/src/services/hid.rs index 6ad67fb..bbeefb4 100644 --- a/ctru-rs/src/services/hid.rs +++ b/ctru-rs/src/services/hid.rs @@ -164,7 +164,7 @@ impl Drop for Hid { fn drop(&mut self) { unsafe { ctru_sys::hidExit() }; - HID_ACTIVE.store(false, Ordering::Release); + HID_ACTIVE.store(false, Ordering::SeqCst); } } @@ -175,6 +175,9 @@ mod tests { #[test] fn hid_duplicate() { // We don't need to build a `Hid` because the test runner has one already - assert!(Hid::init().is_err()); + match Hid::init() { + Err(Error::ServiceAlreadyActive("Hid")) => return, + _ => panic!(), + } } } diff --git a/ctru-rs/src/services/soc.rs b/ctru-rs/src/services/soc.rs index a15541a..3bad215 100644 --- a/ctru-rs/src/services/soc.rs +++ b/ctru-rs/src/services/soc.rs @@ -61,7 +61,7 @@ impl Drop for Soc { free(self.soc_mem as *mut _); } - SOC_ACTIVE.store(false, Ordering::Release); + SOC_ACTIVE.store(false, Ordering::SeqCst); } } @@ -73,6 +73,9 @@ mod tests { fn soc_duplicate() { let _soc = Soc::init().unwrap(); - assert!(Soc::init().is_err()); + match Soc::init() { + Err(Error::ServiceAlreadyActive("Soc")) => return, + _ => panic!(), + } } } diff --git a/ctru-rs/src/services/sslc.rs b/ctru-rs/src/services/sslc.rs index 0b2192e..4778819 100644 --- a/ctru-rs/src/services/sslc.rs +++ b/ctru-rs/src/services/sslc.rs @@ -40,7 +40,7 @@ impl Drop for SslC { fn drop(&mut self) { unsafe { ctru_sys::sslcExit() }; - SSLC_ACTIVE.store(false, Ordering::Release); + SSLC_ACTIVE.store(false, Ordering::SeqCst); } } @@ -52,7 +52,9 @@ mod tests { fn sslc_duplicate() { let _sslc = SslC::init().unwrap(); - assert!(SslC::init().is_err()); + match SslC::init() { + Err(Error::ServiceAlreadyActive("SslC")) => return, + _ => panic!(), + } } } - diff --git a/ctru-rs/src/srv.rs b/ctru-rs/src/srv.rs index 8cca5c3..d817b94 100644 --- a/ctru-rs/src/srv.rs +++ b/ctru-rs/src/srv.rs @@ -27,7 +27,7 @@ impl Drop for Srv { fn drop(&mut self) { unsafe { ctru_sys::srvExit() }; - SRV_ACTIVE.store(false, Ordering::Release); + SRV_ACTIVE.store(false, Ordering::SeqCst); } } @@ -39,6 +39,9 @@ mod tests { fn srv_duplicate() { let _srv = Srv::init().unwrap(); - assert!(Srv::init().is_err()); + match Srv::init() { + Err(Error::ServiceAlreadyActive("Srv")) => return, + _ => panic!(), + } } } diff --git a/ctru-rs/src/test_runner.rs b/ctru-rs/src/test_runner.rs index 0e9ab06..ab4c60f 100644 --- a/ctru-rs/src/test_runner.rs +++ b/ctru-rs/src/test_runner.rs @@ -17,7 +17,7 @@ use crate::services::Apt; pub(crate) fn run(tests: &[&TestDescAndFn]) { crate::init(); - let gfx = Gfx::init_default().unwrap(); + let gfx = Gfx::init().unwrap(); let hid = Hid::init().unwrap(); let apt = Apt::init().unwrap(); From dda1706dc935362816d3be3ddfd8be0b4c8968af Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Thu, 10 Mar 2022 19:20:22 +0100 Subject: [PATCH 3/9] Missed ordering changes --- ctru-rs/src/gfx.rs | 2 +- ctru-rs/src/romfs.rs | 2 +- ctru-rs/src/services/apt.rs | 2 +- ctru-rs/src/services/fs.rs | 2 +- ctru-rs/src/services/hid.rs | 2 +- ctru-rs/src/services/soc.rs | 2 +- ctru-rs/src/services/sslc.rs | 2 +- ctru-rs/src/srv.rs | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index b03e64d..0d244c1 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -86,7 +86,7 @@ impl Gfx { bottom_fb_fmt: FramebufferFormat, use_vram_buffers: bool, ) -> Result { - match GFX_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match GFX_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => { unsafe { ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), use_vram_buffers); diff --git a/ctru-rs/src/romfs.rs b/ctru-rs/src/romfs.rs index f09c25e..78005fc 100644 --- a/ctru-rs/src/romfs.rs +++ b/ctru-rs/src/romfs.rs @@ -22,7 +22,7 @@ static ROMFS_ACTIVE: AtomicBool = AtomicBool::new(false); impl RomFS { pub fn init() -> crate::Result { - match ROMFS_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match ROMFS_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => { let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); let result = unsafe { ctru_sys::romfsMountSelf(mount_name.as_ptr()) }; diff --git a/ctru-rs/src/services/apt.rs b/ctru-rs/src/services/apt.rs index 85fb710..29ab58c 100644 --- a/ctru-rs/src/services/apt.rs +++ b/ctru-rs/src/services/apt.rs @@ -9,7 +9,7 @@ static APT_ACTIVE: AtomicBool = AtomicBool::new(false); impl Apt { pub fn init() -> crate::Result { - match APT_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match APT_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => { let r = unsafe { ctru_sys::aptInit() }; if r < 0 { diff --git a/ctru-rs/src/services/fs.rs b/ctru-rs/src/services/fs.rs index b4c7cfa..cd8fc20 100644 --- a/ctru-rs/src/services/fs.rs +++ b/ctru-rs/src/services/fs.rs @@ -308,7 +308,7 @@ impl Fs { /// as many times as desired and the service will not exit until all /// instances of Fs drop out of scope. pub fn init() -> crate::Result { - match FS_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match FS_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => { let r = unsafe { ctru_sys::fsInit() }; if r < 0 { diff --git a/ctru-rs/src/services/hid.rs b/ctru-rs/src/services/hid.rs index bbeefb4..567849f 100644 --- a/ctru-rs/src/services/hid.rs +++ b/ctru-rs/src/services/hid.rs @@ -70,7 +70,7 @@ pub struct CirclePosition(ctru_sys::circlePosition); /// rare in practice. impl Hid { pub fn init() -> crate::Result { - match HID_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match HID_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => { let r = unsafe { ctru_sys::hidInit() }; if r < 0 { diff --git a/ctru-rs/src/services/soc.rs b/ctru-rs/src/services/soc.rs index 3bad215..e04e040 100644 --- a/ctru-rs/src/services/soc.rs +++ b/ctru-rs/src/services/soc.rs @@ -31,7 +31,7 @@ impl Soc { /// /// This function will return an error if the `Soc` service is already initialized pub fn init_with_buffer_size(num_bytes: usize) -> crate::Result { - match SOC_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match SOC_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => unsafe { let soc_mem = memalign(0x1000, num_bytes) as *mut u32; diff --git a/ctru-rs/src/services/sslc.rs b/ctru-rs/src/services/sslc.rs index 4778819..0ee2661 100644 --- a/ctru-rs/src/services/sslc.rs +++ b/ctru-rs/src/services/sslc.rs @@ -12,7 +12,7 @@ static SSLC_ACTIVE: AtomicBool = AtomicBool::new(false); impl SslC { /// Initialize sslc pub fn init() -> crate::Result { - match SSLC_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match SSLC_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => { let r = unsafe { ctru_sys::sslcInit(0) }; if r < 0 { diff --git a/ctru-rs/src/srv.rs b/ctru-rs/src/srv.rs index d817b94..5445066 100644 --- a/ctru-rs/src/srv.rs +++ b/ctru-rs/src/srv.rs @@ -9,7 +9,7 @@ static SRV_ACTIVE: AtomicBool = AtomicBool::new(false); impl Srv { pub fn init() -> crate::Result { - match SRV_ACTIVE.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { + match SRV_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => { let r = unsafe { ctru_sys::srvInit() }; if r < 0 { From 9da34bd050d9844a4a6fc4d4b221134bcd9d86ae Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Sat, 12 Mar 2022 22:59:50 +0100 Subject: [PATCH 4/9] Using Mutexes and ServiceHandler --- ctru-rs/src/error.rs | 9 ++--- ctru-rs/src/gfx.rs | 50 ++++++++++++--------------- ctru-rs/src/lib.rs | 3 ++ ctru-rs/src/romfs.rs | 56 ++++++++++++++++--------------- ctru-rs/src/services/apt.rs | 52 ++++++++++++++++------------- ctru-rs/src/services/fs.rs | 62 +++++++++++++++++++--------------- ctru-rs/src/services/hid.rs | 48 +++++++++++++------------- ctru-rs/src/services/mod.rs | 45 +++++++++++++++++++++++++ ctru-rs/src/services/soc.rs | 65 ++++++++++++++++++------------------ ctru-rs/src/services/sslc.rs | 58 +++++++++++++++++++------------- ctru-rs/src/srv.rs | 50 +++++++++++++++------------ 11 files changed, 285 insertions(+), 213 deletions(-) diff --git a/ctru-rs/src/error.rs b/ctru-rs/src/error.rs index 2cfe55a..96ade26 100644 --- a/ctru-rs/src/error.rs +++ b/ctru-rs/src/error.rs @@ -9,7 +9,7 @@ pub type Result = ::std::result::Result; #[non_exhaustive] pub enum Error { Os(ctru_sys::Result), - ServiceAlreadyActive(&'static str), + ServiceAlreadyActive, } impl From for Error { @@ -29,10 +29,7 @@ impl fmt::Debug for Error { .field("summary", &R_SUMMARY(err)) .field("level", &R_LEVEL(err)) .finish(), - Error::ServiceAlreadyActive(service) => f - .debug_tuple("ServiceAlreadyActive") - .field(&String::from(service)) - .finish(), + Error::ServiceAlreadyActive => f.debug_tuple("ServiceAlreadyActive").finish(), } } } @@ -44,7 +41,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { Error::Os(err) => write!(f, "libctru result code: 0x{:08X}", err), - Error::ServiceAlreadyActive(service) => write!(f, "Service {service} already active"), + Error::ServiceAlreadyActive => write!(f, "Service already active"), } } } diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index 0d244c1..4016a91 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -1,12 +1,13 @@ //! LCD screens manipulation helper use std::cell::RefCell; +use std::lazy::SyncLazy; use std::marker::PhantomData; -use std::ops::Drop; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Mutex; use crate::error::{Error, Result}; use crate::services::gspgpu::{self, FramebufferFormat}; +use crate::services::ServiceHandler; /// Trait implemented by TopScreen and BottomScreen for common methods pub trait Screen { @@ -72,9 +73,10 @@ pub enum Side { pub struct Gfx { pub top_screen: RefCell, pub bottom_screen: RefCell, + _service_handler: ServiceHandler, } -static GFX_ACTIVE: AtomicBool = AtomicBool::new(false); +static GFX_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); impl Gfx { /// Initialize the Gfx module with the chosen framebuffer formats for the top and bottom @@ -86,19 +88,22 @@ impl Gfx { bottom_fb_fmt: FramebufferFormat, use_vram_buffers: bool, ) -> Result { - match GFX_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => { - unsafe { - ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), use_vram_buffers); - } - - Ok(Gfx { - top_screen: RefCell::new(TopScreen), - bottom_screen: RefCell::new(BottomScreen), - }) - } - Err(_) => Err(Error::ServiceAlreadyActive("Gfx")), - } + let _service_handler = ServiceHandler::new( + &GFX_ACTIVE, + false, + || unsafe { + ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), use_vram_buffers); + + Ok(()) + }, + || unsafe { ctru_sys::gfxExit() }, + )?; + + Ok(Self { + top_screen: RefCell::new(TopScreen), + bottom_screen: RefCell::new(BottomScreen), + _service_handler, + }) } /// Creates a new Gfx instance with default init values @@ -212,14 +217,6 @@ impl From for ctru_sys::gfx3dSide_t { } } -impl Drop for Gfx { - fn drop(&mut self) { - unsafe { ctru_sys::gfxExit() }; - - GFX_ACTIVE.store(false, Ordering::SeqCst); - } -} - #[cfg(test)] mod tests { use super::*; @@ -227,9 +224,6 @@ mod tests { #[test] fn gfx_duplicate() { // We don't need to build a `Gfx` because the test runner has one already - match Gfx::init() { - Err(Error::ServiceAlreadyActive("Gfx")) => return, - _ => panic!(), - } + assert!(matches!(Gfx::init(), Err(Error::ServiceAlreadyActive))) } } diff --git a/ctru-rs/src/lib.rs b/ctru-rs/src/lib.rs index 8cad7a9..559f2db 100644 --- a/ctru-rs/src/lib.rs +++ b/ctru-rs/src/lib.rs @@ -3,6 +3,7 @@ #![feature(test)] #![feature(custom_test_frameworks)] #![test_runner(test_runner::run)] +#![feature(once_cell)] extern "C" fn services_deinit() { unsafe { @@ -28,6 +29,7 @@ pub fn init() { use std::panic::PanicInfo; + #[cfg(not(test))] let main_thread = thread::current().id(); // Panic Hook setup @@ -36,6 +38,7 @@ pub fn init() { default_hook(info); // Only for panics in the main thread + #[cfg(not(test))] if main_thread == thread::current().id() && console::Console::exists() { println!("\nPress SELECT to exit the software"); diff --git a/ctru-rs/src/romfs.rs b/ctru-rs/src/romfs.rs index 78005fc..0d366c8 100644 --- a/ctru-rs/src/romfs.rs +++ b/ctru-rs/src/romfs.rs @@ -11,39 +11,39 @@ //! ``` use std::ffi::CStr; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::lazy::SyncLazy; +use std::sync::Mutex; -use crate::error::Error; +use crate::services::ServiceHandler; #[non_exhaustive] -pub struct RomFS; +pub struct RomFS { + _service_handler: ServiceHandler, +} -static ROMFS_ACTIVE: AtomicBool = AtomicBool::new(false); +static ROMFS_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); impl RomFS { pub fn init() -> crate::Result { - match ROMFS_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => { + let _service_handler = ServiceHandler::new( + &ROMFS_ACTIVE, + true, + || { let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); - let result = unsafe { ctru_sys::romfsMountSelf(mount_name.as_ptr()) }; - - if result < 0 { - Err(result.into()) - } else { - Ok(Self) + let r = unsafe { ctru_sys::romfsMountSelf(mount_name.as_ptr()) }; + if r < 0 { + return Err(r.into()); } - } - Err(_) => Err(Error::ServiceAlreadyActive("RomFS")), - } - } -} -impl Drop for RomFS { - fn drop(&mut self) { - let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); - unsafe { ctru_sys::romfsUnmount(mount_name.as_ptr()) }; + Ok(()) + }, + || { + let mount_name = CStr::from_bytes_with_nul(b"romfs\0").unwrap(); + unsafe { ctru_sys::romfsUnmount(mount_name.as_ptr()) }; + }, + )?; - ROMFS_ACTIVE.store(false, Ordering::SeqCst); + Ok(Self { _service_handler }) } } @@ -54,10 +54,14 @@ mod tests { #[test] fn romfs_duplicate() { let _romfs = RomFS::init().unwrap(); + let lock = *ROMFS_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 1); + + drop(_romfs); + + let lock = *ROMFS_ACTIVE.lock().unwrap(); - match RomFS::init() { - Err(Error::ServiceAlreadyActive("RomFS")) => return, - _ => panic!(), - } + assert_eq!(lock, 0); } } diff --git a/ctru-rs/src/services/apt.rs b/ctru-rs/src/services/apt.rs index 29ab58c..089b896 100644 --- a/ctru-rs/src/services/apt.rs +++ b/ctru-rs/src/services/apt.rs @@ -1,25 +1,38 @@ -use std::sync::atomic::{AtomicBool, Ordering}; +use std::lazy::SyncLazy; +use std::sync::Mutex; -use crate::error::Error; +use crate::services::ServiceHandler; #[non_exhaustive] -pub struct Apt(()); +pub struct Apt { + _service_handler: ServiceHandler, +} -static APT_ACTIVE: AtomicBool = AtomicBool::new(false); +static APT_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); impl Apt { pub fn init() -> crate::Result { - match APT_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => { + let _service_handler = ServiceHandler::new( + &APT_ACTIVE, + true, + || { let r = unsafe { ctru_sys::aptInit() }; if r < 0 { - Err(r.into()) - } else { - Ok(Self(())) + return Err(r.into()); } - } - Err(_) => Err(Error::ServiceAlreadyActive("Apt")), - } + + Ok(()) + }, + // `socExit` returns an error code. There is no documentantion of when errors could happen, + // but we wouldn't be able to handle them in the `Drop` implementation anyways. + // Surely nothing bad will happens :D + || unsafe { + // The socket buffer is freed automatically by `socExit` + ctru_sys::aptExit(); + }, + )?; + + Ok(Self { _service_handler }) } pub fn main_loop(&self) -> bool { @@ -36,14 +49,6 @@ impl Apt { } } -impl Drop for Apt { - fn drop(&mut self) { - unsafe { ctru_sys::aptExit() }; - - APT_ACTIVE.store(false, Ordering::SeqCst); - } -} - #[cfg(test)] mod tests { use super::*; @@ -51,9 +56,8 @@ mod tests { #[test] fn apt_duplicate() { // We don't need to build a `Apt` because the test runner has one already - match Apt::init() { - Err(Error::ServiceAlreadyActive("Apt")) => return, - _ => panic!(), - } + let lock = *APT_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 1); } } diff --git a/ctru-rs/src/services/fs.rs b/ctru-rs/src/services/fs.rs index cd8fc20..731e1e0 100644 --- a/ctru-rs/src/services/fs.rs +++ b/ctru-rs/src/services/fs.rs @@ -9,15 +9,16 @@ use std::io::Error as IoError; use std::io::ErrorKind as IoErrorKind; use std::io::Result as IoResult; use std::io::{Read, Seek, SeekFrom, Write}; +use std::lazy::SyncLazy; use std::mem; use std::path::{Path, PathBuf}; use std::ptr; use std::slice; -use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; + use widestring::{WideCStr, WideCString}; -use crate::error::Error; +use crate::services::ServiceHandler; bitflags! { #[derive(Default)] @@ -86,9 +87,11 @@ pub enum ArchiveID { /// /// The service exits when all instances of this struct go out of scope. #[non_exhaustive] -pub struct Fs(()); +pub struct Fs { + _service_handler: ServiceHandler, +} -static FS_ACTIVE: AtomicBool = AtomicBool::new(false); +static FS_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); /// Handle to an open filesystem archive. /// @@ -308,17 +311,27 @@ impl Fs { /// as many times as desired and the service will not exit until all /// instances of Fs drop out of scope. pub fn init() -> crate::Result { - match FS_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => { + let _service_handler = ServiceHandler::new( + &FS_ACTIVE, + true, + || { let r = unsafe { ctru_sys::fsInit() }; if r < 0 { - Err(r.into()) - } else { - Ok(Self(())) + return Err(r.into()); } - } - Err(_) => Err(Error::ServiceAlreadyActive("Fs")), - } + + Ok(()) + }, + // `socExit` returns an error code. There is no documentantion of when errors could happen, + // but we wouldn't be able to handle them in the `Drop` implementation anyways. + // Surely nothing bad will happens :D + || unsafe { + // The socket buffer is freed automatically by `socExit` + ctru_sys::fsExit(); + }, + )?; + + Ok(Self { _service_handler }) } /// Returns a handle to the SDMC (memory card) Archive. @@ -998,16 +1011,6 @@ impl Seek for File { } } -impl Drop for Fs { - fn drop(&mut self) { - unsafe { - ctru_sys::fsExit(); - } - - FS_ACTIVE.store(false, Ordering::SeqCst); - } -} - impl Drop for Archive { fn drop(&mut self) { unsafe { @@ -1083,9 +1086,14 @@ mod tests { fn fs_duplicate() { let _fs = Fs::init().unwrap(); - match Fs::init() { - Err(Error::ServiceAlreadyActive("Fs")) => return, - _ => panic!(), - } + let lock = *FS_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 1); + + drop(_fs); + + let lock = *FS_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 0); } } diff --git a/ctru-rs/src/services/hid.rs b/ctru-rs/src/services/hid.rs index 567849f..c26e96a 100644 --- a/ctru-rs/src/services/hid.rs +++ b/ctru-rs/src/services/hid.rs @@ -4,9 +4,10 @@ //! and circle pad information. It also provides information from the sound volume slider, //! the accelerometer, and the gyroscope. -use std::sync::atomic::{AtomicBool, Ordering}; +use std::lazy::SyncLazy; +use std::sync::Mutex; -use crate::error::Error; +use crate::services::ServiceHandler; bitflags::bitflags! { /// A set of flags corresponding to the button and directional pad @@ -49,9 +50,11 @@ bitflags::bitflags! { /// /// This service requires no special permissions to use. #[non_exhaustive] -pub struct Hid(()); +pub struct Hid { + _service_handler: ServiceHandler, +} -static HID_ACTIVE: AtomicBool = AtomicBool::new(false); +static HID_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); /// Represents user input to the touchscreen. #[non_exhaustive] @@ -70,17 +73,23 @@ pub struct CirclePosition(ctru_sys::circlePosition); /// rare in practice. impl Hid { pub fn init() -> crate::Result { - match HID_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => { + let _service_handler = ServiceHandler::new( + &HID_ACTIVE, + true, + || { let r = unsafe { ctru_sys::hidInit() }; if r < 0 { - Err(r.into()) - } else { - Ok(Self(())) + return Err(r.into()); } - } - Err(_) => Err(Error::ServiceAlreadyActive("Hid")), - } + + Ok(()) + }, + || unsafe { + ctru_sys::hidExit(); + }, + )?; + + Ok(Self { _service_handler }) } /// Scans the HID service for all user input occurring on the current @@ -160,14 +169,6 @@ impl CirclePosition { } } -impl Drop for Hid { - fn drop(&mut self) { - unsafe { ctru_sys::hidExit() }; - - HID_ACTIVE.store(false, Ordering::SeqCst); - } -} - #[cfg(test)] mod tests { use super::*; @@ -175,9 +176,8 @@ mod tests { #[test] fn hid_duplicate() { // We don't need to build a `Hid` because the test runner has one already - match Hid::init() { - Err(Error::ServiceAlreadyActive("Hid")) => return, - _ => panic!(), - } + let lock = *HID_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 1); } } diff --git a/ctru-rs/src/services/mod.rs b/ctru-rs/src/services/mod.rs index 2159e9e..67418de 100644 --- a/ctru-rs/src/services/mod.rs +++ b/ctru-rs/src/services/mod.rs @@ -9,3 +9,48 @@ pub mod sslc; pub use self::apt::Apt; pub use self::hid::Hid; pub use self::sslc::SslC; + +use crate::Error; +use std::sync::Mutex; +pub(crate) struct ServiceHandler { + counter: &'static Mutex, + close: Box, +} + +impl ServiceHandler { + pub fn new( + counter: &'static Mutex, + allow_multiple: bool, + start: S, + close: E, + ) -> crate::Result + where + S: FnOnce() -> crate::Result<()>, + E: Fn() + 'static, + { + let mut value = counter.lock().unwrap(); // todo: handle poisoning + + if *value == 0 { + start()?; + } else if !allow_multiple { + return Err(Error::ServiceAlreadyActive); + } + + *value += 1; + + Ok(Self { + counter, + close: Box::new(close), + }) + } +} + +impl Drop for ServiceHandler { + fn drop(&mut self) { + let mut value = self.counter.lock().unwrap(); // should probably handle poisoning - could just map_err to ignore it. + *value -= 1; + if *value == 0 { + (self.close)(); + } + } +} diff --git a/ctru-rs/src/services/soc.rs b/ctru-rs/src/services/soc.rs index e04e040..44041f4 100644 --- a/ctru-rs/src/services/soc.rs +++ b/ctru-rs/src/services/soc.rs @@ -1,18 +1,18 @@ -use libc::{free, memalign}; +use libc::memalign; +use std::lazy::SyncLazy; use std::net::Ipv4Addr; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Mutex; -use crate::error::Error; -use ctru_sys::{socExit, socInit}; +use crate::services::ServiceHandler; /// Soc service. Initializing this service will enable the use of network sockets and utilities /// such as those found in `std::net`. The service will be closed when this struct is is dropped. #[non_exhaustive] pub struct Soc { - soc_mem: *mut u32, + _service_handler: ServiceHandler, } -static SOC_ACTIVE: AtomicBool = AtomicBool::new(false); +static SOC_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); impl Soc { /// Initialize the Soc service with a default buffer size of 0x100000 bytes @@ -31,20 +31,28 @@ impl Soc { /// /// This function will return an error if the `Soc` service is already initialized pub fn init_with_buffer_size(num_bytes: usize) -> crate::Result { - match SOC_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => unsafe { - let soc_mem = memalign(0x1000, num_bytes) as *mut u32; - - let r = socInit(soc_mem, num_bytes as u32); + let _service_handler = ServiceHandler::new( + &SOC_ACTIVE, + true, + || { + let soc_mem = unsafe { memalign(0x1000, num_bytes) } as *mut u32; + let r = unsafe { ctru_sys::socInit(soc_mem, num_bytes as u32) }; if r < 0 { - free(soc_mem as *mut _); - Err(r.into()) - } else { - Ok(Self { soc_mem }) + return Err(r.into()); } + + Ok(()) + }, + // `socExit` returns an error code. There is no documentantion of when errors could happen, + // but we wouldn't be able to handle them in the `Drop` implementation anyways. + // Surely nothing bad will happens :D + || unsafe { + // The socket buffer is freed automatically by `socExit` + ctru_sys::socExit(); }, - Err(_) => Err(Error::ServiceAlreadyActive("Soc")), - } + )?; + + Ok(Self { _service_handler }) } /// IP Address of the Nintendo 3DS system. @@ -54,17 +62,6 @@ impl Soc { } } -impl Drop for Soc { - fn drop(&mut self) { - unsafe { - socExit(); - free(self.soc_mem as *mut _); - } - - SOC_ACTIVE.store(false, Ordering::SeqCst); - } -} - #[cfg(test)] mod tests { use super::*; @@ -72,10 +69,14 @@ mod tests { #[test] fn soc_duplicate() { let _soc = Soc::init().unwrap(); + let lock = *SOC_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 1); + + drop(_soc); + + let lock = *SOC_ACTIVE.lock().unwrap(); - match Soc::init() { - Err(Error::ServiceAlreadyActive("Soc")) => return, - _ => panic!(), - } + assert_eq!(lock, 0); } } diff --git a/ctru-rs/src/services/sslc.rs b/ctru-rs/src/services/sslc.rs index 0ee2661..ba8d41f 100644 --- a/ctru-rs/src/services/sslc.rs +++ b/ctru-rs/src/services/sslc.rs @@ -1,28 +1,41 @@ // TODO: Implement remaining functions -use std::sync::atomic::{AtomicBool, Ordering}; +use std::lazy::SyncLazy; +use std::sync::Mutex; -use crate::error::Error; +use crate::services::ServiceHandler; #[non_exhaustive] -pub struct SslC(()); +pub struct SslC { + _service_handler: ServiceHandler, +} -static SSLC_ACTIVE: AtomicBool = AtomicBool::new(false); +static SSLC_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); impl SslC { /// Initialize sslc pub fn init() -> crate::Result { - match SSLC_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => { + let _service_handler = ServiceHandler::new( + &SSLC_ACTIVE, + true, + || { let r = unsafe { ctru_sys::sslcInit(0) }; if r < 0 { - Err(r.into()) - } else { - Ok(Self(())) + return Err(r.into()); } - } - Err(_) => Err(Error::ServiceAlreadyActive("SslC")), - } + + Ok(()) + }, + // `socExit` returns an error code. There is no documentantion of when errors could happen, + // but we wouldn't be able to handle them in the `Drop` implementation anyways. + // Surely nothing bad will happens :D + || unsafe { + // The socket buffer is freed automatically by `socExit` + ctru_sys::sslcExit(); + }, + )?; + + Ok(Self { _service_handler }) } /// Fill `buf` with `buf.len()` random bytes @@ -36,14 +49,6 @@ impl SslC { } } -impl Drop for SslC { - fn drop(&mut self) { - unsafe { ctru_sys::sslcExit() }; - - SSLC_ACTIVE.store(false, Ordering::SeqCst); - } -} - #[cfg(test)] mod tests { use super::*; @@ -52,9 +57,14 @@ mod tests { fn sslc_duplicate() { let _sslc = SslC::init().unwrap(); - match SslC::init() { - Err(Error::ServiceAlreadyActive("SslC")) => return, - _ => panic!(), - } + let lock = *SSLC_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 1); + + drop(_sslc); + + let lock = *SSLC_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 0); } } diff --git a/ctru-rs/src/srv.rs b/ctru-rs/src/srv.rs index 5445066..fa0f353 100644 --- a/ctru-rs/src/srv.rs +++ b/ctru-rs/src/srv.rs @@ -1,33 +1,34 @@ -use std::sync::atomic::{AtomicBool, Ordering}; +use std::lazy::SyncLazy; +use std::sync::Mutex; -use crate::error::Error; +use crate::services::ServiceHandler; #[non_exhaustive] -pub struct Srv(()); +pub struct Srv { + _service_handler: ServiceHandler, +} -static SRV_ACTIVE: AtomicBool = AtomicBool::new(false); +static SRV_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); impl Srv { pub fn init() -> crate::Result { - match SRV_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => { + let _service_handler = ServiceHandler::new( + &SRV_ACTIVE, + true, + || { let r = unsafe { ctru_sys::srvInit() }; if r < 0 { - Err(r.into()) - } else { - Ok(Self(())) + return Err(r.into()); } - } - Err(_) => Err(Error::ServiceAlreadyActive("Srv")), - } - } -} -impl Drop for Srv { - fn drop(&mut self) { - unsafe { ctru_sys::srvExit() }; + Ok(()) + }, + || unsafe { + ctru_sys::srvExit(); + }, + )?; - SRV_ACTIVE.store(false, Ordering::SeqCst); + Ok(Self { _service_handler }) } } @@ -39,9 +40,14 @@ mod tests { fn srv_duplicate() { let _srv = Srv::init().unwrap(); - match Srv::init() { - Err(Error::ServiceAlreadyActive("Srv")) => return, - _ => panic!(), - } + let lock = *SRV_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 1); + + drop(_srv); + + let lock = *SRV_ACTIVE.lock().unwrap(); + + assert_eq!(lock, 0); } } From 6f15e54da9d65f6b61ed5fa6d02edc4b990f0ad5 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Sun, 13 Mar 2022 12:19:25 +0100 Subject: [PATCH 5/9] Fixed nits and such --- ctru-rs/Cargo.toml | 1 + ctru-rs/src/gfx.rs | 10 ++++---- ctru-rs/src/lib.rs | 8 ++++--- ctru-rs/src/romfs.rs | 18 +++++++------- ctru-rs/src/services/apt.rs | 18 ++++++-------- ctru-rs/src/services/fs.rs | 22 +++++++---------- ctru-rs/src/services/hid.rs | 14 +++++------ ctru-rs/src/services/mod.rs | 46 ++---------------------------------- ctru-rs/src/services/soc.rs | 18 +++++++------- ctru-rs/src/services/sslc.rs | 22 +++++++---------- ctru-rs/src/srv.rs | 18 +++++++------- 11 files changed, 72 insertions(+), 123 deletions(-) diff --git a/ctru-rs/Cargo.toml b/ctru-rs/Cargo.toml index abc6f79..53d39d3 100644 --- a/ctru-rs/Cargo.toml +++ b/ctru-rs/Cargo.toml @@ -29,6 +29,7 @@ futures = "0.3" time = "0.3.7" tokio = { version = "1.16", features = ["rt", "time", "sync", "macros"] } cfg-if = "1.0.0" +once_cell = "1.10.0" [features] default = ["romfs"] diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index 4016a91..44104f8 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -1,13 +1,13 @@ //! LCD screens manipulation helper use std::cell::RefCell; -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::marker::PhantomData; use std::sync::Mutex; use crate::error::{Error, Result}; use crate::services::gspgpu::{self, FramebufferFormat}; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; /// Trait implemented by TopScreen and BottomScreen for common methods pub trait Screen { @@ -73,10 +73,10 @@ pub enum Side { pub struct Gfx { pub top_screen: RefCell, pub bottom_screen: RefCell, - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static GFX_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static GFX_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); impl Gfx { /// Initialize the Gfx module with the chosen framebuffer formats for the top and bottom @@ -88,7 +88,7 @@ impl Gfx { bottom_fb_fmt: FramebufferFormat, use_vram_buffers: bool, ) -> Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &GFX_ACTIVE, false, || unsafe { diff --git a/ctru-rs/src/lib.rs b/ctru-rs/src/lib.rs index 559f2db..40acf84 100644 --- a/ctru-rs/src/lib.rs +++ b/ctru-rs/src/lib.rs @@ -3,7 +3,6 @@ #![feature(test)] #![feature(custom_test_frameworks)] #![test_runner(test_runner::run)] -#![feature(once_cell)] extern "C" fn services_deinit() { unsafe { @@ -27,9 +26,13 @@ pub fn init() { libc::atexit(services_deinit); } + #[cfg(test)] + panic_hook_setup(); +} + +fn panic_hook_setup() { use std::panic::PanicInfo; - #[cfg(not(test))] let main_thread = thread::current().id(); // Panic Hook setup @@ -38,7 +41,6 @@ pub fn init() { default_hook(info); // Only for panics in the main thread - #[cfg(not(test))] if main_thread == thread::current().id() && console::Console::exists() { println!("\nPress SELECT to exit the software"); diff --git a/ctru-rs/src/romfs.rs b/ctru-rs/src/romfs.rs index 0d366c8..c802706 100644 --- a/ctru-rs/src/romfs.rs +++ b/ctru-rs/src/romfs.rs @@ -11,21 +11,21 @@ //! ``` use std::ffi::CStr; -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::sync::Mutex; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; #[non_exhaustive] pub struct RomFS { - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static ROMFS_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static ROMFS_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); impl RomFS { pub fn init() -> crate::Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &ROMFS_ACTIVE, true, || { @@ -54,14 +54,14 @@ mod tests { #[test] fn romfs_duplicate() { let _romfs = RomFS::init().unwrap(); - let lock = *ROMFS_ACTIVE.lock().unwrap(); + let value = *ROMFS_ACTIVE.lock().unwrap(); - assert_eq!(lock, 1); + assert_eq!(value, 1); drop(_romfs); - let lock = *ROMFS_ACTIVE.lock().unwrap(); + let value = *ROMFS_ACTIVE.lock().unwrap(); - assert_eq!(lock, 0); + assert_eq!(value, 0); } } diff --git a/ctru-rs/src/services/apt.rs b/ctru-rs/src/services/apt.rs index 089b896..eb53fe9 100644 --- a/ctru-rs/src/services/apt.rs +++ b/ctru-rs/src/services/apt.rs @@ -1,18 +1,18 @@ -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::sync::Mutex; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; #[non_exhaustive] pub struct Apt { - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static APT_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static APT_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); impl Apt { pub fn init() -> crate::Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &APT_ACTIVE, true, || { @@ -23,11 +23,7 @@ impl Apt { Ok(()) }, - // `socExit` returns an error code. There is no documentantion of when errors could happen, - // but we wouldn't be able to handle them in the `Drop` implementation anyways. - // Surely nothing bad will happens :D || unsafe { - // The socket buffer is freed automatically by `socExit` ctru_sys::aptExit(); }, )?; @@ -56,8 +52,8 @@ mod tests { #[test] fn apt_duplicate() { // We don't need to build a `Apt` because the test runner has one already - let lock = *APT_ACTIVE.lock().unwrap(); + let value = *APT_ACTIVE.lock().unwrap(); - assert_eq!(lock, 1); + assert_eq!(value, 1); } } diff --git a/ctru-rs/src/services/fs.rs b/ctru-rs/src/services/fs.rs index 731e1e0..2c70195 100644 --- a/ctru-rs/src/services/fs.rs +++ b/ctru-rs/src/services/fs.rs @@ -9,7 +9,7 @@ use std::io::Error as IoError; use std::io::ErrorKind as IoErrorKind; use std::io::Result as IoResult; use std::io::{Read, Seek, SeekFrom, Write}; -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::mem; use std::path::{Path, PathBuf}; use std::ptr; @@ -18,7 +18,7 @@ use std::sync::{Arc, Mutex}; use widestring::{WideCStr, WideCString}; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; bitflags! { #[derive(Default)] @@ -88,10 +88,10 @@ pub enum ArchiveID { /// The service exits when all instances of this struct go out of scope. #[non_exhaustive] pub struct Fs { - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static FS_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static FS_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); /// Handle to an open filesystem archive. /// @@ -311,7 +311,7 @@ impl Fs { /// as many times as desired and the service will not exit until all /// instances of Fs drop out of scope. pub fn init() -> crate::Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &FS_ACTIVE, true, || { @@ -322,11 +322,7 @@ impl Fs { Ok(()) }, - // `socExit` returns an error code. There is no documentantion of when errors could happen, - // but we wouldn't be able to handle them in the `Drop` implementation anyways. - // Surely nothing bad will happens :D || unsafe { - // The socket buffer is freed automatically by `socExit` ctru_sys::fsExit(); }, )?; @@ -1086,14 +1082,14 @@ mod tests { fn fs_duplicate() { let _fs = Fs::init().unwrap(); - let lock = *FS_ACTIVE.lock().unwrap(); + let value = *FS_ACTIVE.lock().unwrap(); - assert_eq!(lock, 1); + assert_eq!(value, 1); drop(_fs); - let lock = *FS_ACTIVE.lock().unwrap(); + let value = *FS_ACTIVE.lock().unwrap(); - assert_eq!(lock, 0); + assert_eq!(value, 0); } } diff --git a/ctru-rs/src/services/hid.rs b/ctru-rs/src/services/hid.rs index c26e96a..53cdf97 100644 --- a/ctru-rs/src/services/hid.rs +++ b/ctru-rs/src/services/hid.rs @@ -4,10 +4,10 @@ //! and circle pad information. It also provides information from the sound volume slider, //! the accelerometer, and the gyroscope. -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::sync::Mutex; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; bitflags::bitflags! { /// A set of flags corresponding to the button and directional pad @@ -51,10 +51,10 @@ bitflags::bitflags! { /// This service requires no special permissions to use. #[non_exhaustive] pub struct Hid { - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static HID_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static HID_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); /// Represents user input to the touchscreen. #[non_exhaustive] @@ -73,7 +73,7 @@ pub struct CirclePosition(ctru_sys::circlePosition); /// rare in practice. impl Hid { pub fn init() -> crate::Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &HID_ACTIVE, true, || { @@ -176,8 +176,8 @@ mod tests { #[test] fn hid_duplicate() { // We don't need to build a `Hid` because the test runner has one already - let lock = *HID_ACTIVE.lock().unwrap(); + let value = *HID_ACTIVE.lock().unwrap(); - assert_eq!(lock, 1); + assert_eq!(value, 1); } } diff --git a/ctru-rs/src/services/mod.rs b/ctru-rs/src/services/mod.rs index 67418de..cc52685 100644 --- a/ctru-rs/src/services/mod.rs +++ b/ctru-rs/src/services/mod.rs @@ -5,52 +5,10 @@ pub mod hid; pub mod ps; pub mod soc; pub mod sslc; +mod reference; pub use self::apt::Apt; pub use self::hid::Hid; pub use self::sslc::SslC; -use crate::Error; -use std::sync::Mutex; -pub(crate) struct ServiceHandler { - counter: &'static Mutex, - close: Box, -} - -impl ServiceHandler { - pub fn new( - counter: &'static Mutex, - allow_multiple: bool, - start: S, - close: E, - ) -> crate::Result - where - S: FnOnce() -> crate::Result<()>, - E: Fn() + 'static, - { - let mut value = counter.lock().unwrap(); // todo: handle poisoning - - if *value == 0 { - start()?; - } else if !allow_multiple { - return Err(Error::ServiceAlreadyActive); - } - - *value += 1; - - Ok(Self { - counter, - close: Box::new(close), - }) - } -} - -impl Drop for ServiceHandler { - fn drop(&mut self) { - let mut value = self.counter.lock().unwrap(); // should probably handle poisoning - could just map_err to ignore it. - *value -= 1; - if *value == 0 { - (self.close)(); - } - } -} +pub (crate) use self::reference::ServiceReference; diff --git a/ctru-rs/src/services/soc.rs b/ctru-rs/src/services/soc.rs index 44041f4..5514adf 100644 --- a/ctru-rs/src/services/soc.rs +++ b/ctru-rs/src/services/soc.rs @@ -1,18 +1,18 @@ use libc::memalign; -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::net::Ipv4Addr; use std::sync::Mutex; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; /// Soc service. Initializing this service will enable the use of network sockets and utilities /// such as those found in `std::net`. The service will be closed when this struct is is dropped. #[non_exhaustive] pub struct Soc { - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static SOC_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static SOC_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); impl Soc { /// Initialize the Soc service with a default buffer size of 0x100000 bytes @@ -31,7 +31,7 @@ impl Soc { /// /// This function will return an error if the `Soc` service is already initialized pub fn init_with_buffer_size(num_bytes: usize) -> crate::Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &SOC_ACTIVE, true, || { @@ -69,14 +69,14 @@ mod tests { #[test] fn soc_duplicate() { let _soc = Soc::init().unwrap(); - let lock = *SOC_ACTIVE.lock().unwrap(); + let value = *SOC_ACTIVE.lock().unwrap(); - assert_eq!(lock, 1); + assert_eq!(value, 1); drop(_soc); - let lock = *SOC_ACTIVE.lock().unwrap(); + let value = *SOC_ACTIVE.lock().unwrap(); - assert_eq!(lock, 0); + assert_eq!(value, 0); } } diff --git a/ctru-rs/src/services/sslc.rs b/ctru-rs/src/services/sslc.rs index ba8d41f..5c0d0e4 100644 --- a/ctru-rs/src/services/sslc.rs +++ b/ctru-rs/src/services/sslc.rs @@ -1,21 +1,21 @@ // TODO: Implement remaining functions -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::sync::Mutex; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; #[non_exhaustive] pub struct SslC { - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static SSLC_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static SSLC_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); impl SslC { /// Initialize sslc pub fn init() -> crate::Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &SSLC_ACTIVE, true, || { @@ -26,11 +26,7 @@ impl SslC { Ok(()) }, - // `socExit` returns an error code. There is no documentantion of when errors could happen, - // but we wouldn't be able to handle them in the `Drop` implementation anyways. - // Surely nothing bad will happens :D || unsafe { - // The socket buffer is freed automatically by `socExit` ctru_sys::sslcExit(); }, )?; @@ -57,14 +53,14 @@ mod tests { fn sslc_duplicate() { let _sslc = SslC::init().unwrap(); - let lock = *SSLC_ACTIVE.lock().unwrap(); + let value = *SSLC_ACTIVE.lock().unwrap(); - assert_eq!(lock, 1); + assert_eq!(value, 1); drop(_sslc); - let lock = *SSLC_ACTIVE.lock().unwrap(); + let value = *SSLC_ACTIVE.lock().unwrap(); - assert_eq!(lock, 0); + assert_eq!(value, 0); } } diff --git a/ctru-rs/src/srv.rs b/ctru-rs/src/srv.rs index fa0f353..5ab83b6 100644 --- a/ctru-rs/src/srv.rs +++ b/ctru-rs/src/srv.rs @@ -1,18 +1,18 @@ -use std::lazy::SyncLazy; +use once_cell::sync::Lazy; use std::sync::Mutex; -use crate::services::ServiceHandler; +use crate::services::ServiceReference; #[non_exhaustive] pub struct Srv { - _service_handler: ServiceHandler, + _service_handler: ServiceReference, } -static SRV_ACTIVE: SyncLazy> = SyncLazy::new(|| Mutex::new(0)); +static SRV_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); impl Srv { pub fn init() -> crate::Result { - let _service_handler = ServiceHandler::new( + let _service_handler = ServiceReference::new( &SRV_ACTIVE, true, || { @@ -40,14 +40,14 @@ mod tests { fn srv_duplicate() { let _srv = Srv::init().unwrap(); - let lock = *SRV_ACTIVE.lock().unwrap(); + let value = *SRV_ACTIVE.lock().unwrap(); - assert_eq!(lock, 1); + assert_eq!(value, 1); drop(_srv); - let lock = *SRV_ACTIVE.lock().unwrap(); + let value = *SRV_ACTIVE.lock().unwrap(); - assert_eq!(lock, 0); + assert_eq!(value, 0); } } From 9d9dd8f90d754026ccb260892ceae762279966c4 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Mon, 14 Mar 2022 14:17:33 +0100 Subject: [PATCH 6/9] Reverted non-needing services and fixed issues --- ctru-rs/src/gfx.rs | 2 +- ctru-rs/src/lib.rs | 2 +- ctru-rs/src/romfs.rs | 2 +- ctru-rs/src/services/apt.rs | 64 +++++++++------------------ ctru-rs/src/services/fs.rs | 72 ++++++++----------------------- ctru-rs/src/services/hid.rs | 54 ++++++----------------- ctru-rs/src/services/mod.rs | 4 +- ctru-rs/src/services/reference.rs | 44 +++++++++++++++++++ ctru-rs/src/services/sslc.rs | 69 ++++++++--------------------- ctru-rs/src/srv.rs | 59 ++++++------------------- 10 files changed, 133 insertions(+), 239 deletions(-) create mode 100644 ctru-rs/src/services/reference.rs diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index 44104f8..f68f2bc 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -1,7 +1,7 @@ //! LCD screens manipulation helper -use std::cell::RefCell; use once_cell::sync::Lazy; +use std::cell::RefCell; use std::marker::PhantomData; use std::sync::Mutex; diff --git a/ctru-rs/src/lib.rs b/ctru-rs/src/lib.rs index 40acf84..bf3d247 100644 --- a/ctru-rs/src/lib.rs +++ b/ctru-rs/src/lib.rs @@ -26,7 +26,7 @@ pub fn init() { libc::atexit(services_deinit); } - #[cfg(test)] + #[cfg(not(test))] panic_hook_setup(); } diff --git a/ctru-rs/src/romfs.rs b/ctru-rs/src/romfs.rs index c802706..dff45ca 100644 --- a/ctru-rs/src/romfs.rs +++ b/ctru-rs/src/romfs.rs @@ -10,8 +10,8 @@ //! romfs_dir = "romfs" //! ``` -use std::ffi::CStr; use once_cell::sync::Lazy; +use std::ffi::CStr; use std::sync::Mutex; use crate::services::ServiceReference; diff --git a/ctru-rs/src/services/apt.rs b/ctru-rs/src/services/apt.rs index eb53fe9..4662a4e 100644 --- a/ctru-rs/src/services/apt.rs +++ b/ctru-rs/src/services/apt.rs @@ -1,34 +1,15 @@ -use once_cell::sync::Lazy; -use std::sync::Mutex; - -use crate::services::ServiceReference; - -#[non_exhaustive] -pub struct Apt { - _service_handler: ServiceReference, -} - -static APT_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); +pub struct Apt(()); impl Apt { - pub fn init() -> crate::Result { - let _service_handler = ServiceReference::new( - &APT_ACTIVE, - true, - || { - let r = unsafe { ctru_sys::aptInit() }; - if r < 0 { - return Err(r.into()); - } - - Ok(()) - }, - || unsafe { - ctru_sys::aptExit(); - }, - )?; - - Ok(Self { _service_handler }) + pub fn init() -> crate::Result { + unsafe { + let r = ctru_sys::aptInit(); + if r < 0 { + Err(r.into()) + } else { + Ok(Apt(())) + } + } } pub fn main_loop(&self) -> bool { @@ -36,24 +17,19 @@ impl Apt { } pub fn set_app_cpu_time_limit(&self, percent: u32) -> crate::Result<()> { - let r = unsafe { ctru_sys::APT_SetAppCpuTimeLimit(percent) }; - if r < 0 { - Err(r.into()) - } else { - Ok(()) + unsafe { + let r = ctru_sys::APT_SetAppCpuTimeLimit(percent); + if r < 0 { + Err(r.into()) + } else { + Ok(()) + } } } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn apt_duplicate() { - // We don't need to build a `Apt` because the test runner has one already - let value = *APT_ACTIVE.lock().unwrap(); - - assert_eq!(value, 1); +impl Drop for Apt { + fn drop(&mut self) { + unsafe { ctru_sys::aptExit() }; } } diff --git a/ctru-rs/src/services/fs.rs b/ctru-rs/src/services/fs.rs index 2c70195..9f3334a 100644 --- a/ctru-rs/src/services/fs.rs +++ b/ctru-rs/src/services/fs.rs @@ -9,17 +9,13 @@ use std::io::Error as IoError; use std::io::ErrorKind as IoErrorKind; use std::io::Result as IoResult; use std::io::{Read, Seek, SeekFrom, Write}; -use once_cell::sync::Lazy; use std::mem; use std::path::{Path, PathBuf}; use std::ptr; use std::slice; -use std::sync::{Arc, Mutex}; - +use std::sync::Arc; use widestring::{WideCStr, WideCString}; -use crate::services::ServiceReference; - bitflags! { #[derive(Default)] struct FsOpen: u32 { @@ -86,12 +82,7 @@ pub enum ArchiveID { /// until an instance of this struct is created. /// /// The service exits when all instances of this struct go out of scope. -#[non_exhaustive] -pub struct Fs { - _service_handler: ServiceReference, -} - -static FS_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); +pub struct Fs(()); /// Handle to an open filesystem archive. /// @@ -105,7 +96,6 @@ static FS_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); /// let fs = Fs::init().unwrap(); /// let sdmc_archive = fs.sdmc().unwrap(); /// ``` -#[non_exhaustive] pub struct Archive { id: ArchiveID, handle: u64, @@ -174,7 +164,6 @@ pub struct Archive { /// # Ok(()) /// # } /// ``` -#[non_exhaustive] pub struct File { handle: u32, offset: u64, @@ -186,7 +175,6 @@ pub struct File { /// represents known metadata about a file. /// /// [`metadata`]: fn.metadata.html -#[non_exhaustive] pub struct Metadata { attributes: u32, size: u64, @@ -269,7 +257,6 @@ pub struct OpenOptions { /// /// This Result will return Err if there's some sort of intermittent IO error /// during iteration. -#[non_exhaustive] pub struct ReadDir<'a> { handle: Dir, root: Arc, @@ -310,24 +297,15 @@ impl Fs { /// ctrulib services are reference counted, so this function may be called /// as many times as desired and the service will not exit until all /// instances of Fs drop out of scope. - pub fn init() -> crate::Result { - let _service_handler = ServiceReference::new( - &FS_ACTIVE, - true, - || { - let r = unsafe { ctru_sys::fsInit() }; - if r < 0 { - return Err(r.into()); - } - - Ok(()) - }, - || unsafe { - ctru_sys::fsExit(); - }, - )?; - - Ok(Self { _service_handler }) + pub fn init() -> crate::Result { + unsafe { + let r = ctru_sys::fsInit(); + if r < 0 { + Err(r.into()) + } else { + Ok(Fs(())) + } + } } /// Returns a handle to the SDMC (memory card) Archive. @@ -1007,6 +985,14 @@ impl Seek for File { } } +impl Drop for Fs { + fn drop(&mut self) { + unsafe { + ctru_sys::fsExit(); + } + } +} + impl Drop for Archive { fn drop(&mut self) { unsafe { @@ -1073,23 +1059,3 @@ impl From for ctru_sys::FS_ArchiveID { } } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn fs_duplicate() { - let _fs = Fs::init().unwrap(); - - let value = *FS_ACTIVE.lock().unwrap(); - - assert_eq!(value, 1); - - drop(_fs); - - let value = *FS_ACTIVE.lock().unwrap(); - - assert_eq!(value, 0); - } -} diff --git a/ctru-rs/src/services/hid.rs b/ctru-rs/src/services/hid.rs index 53cdf97..18790f1 100644 --- a/ctru-rs/src/services/hid.rs +++ b/ctru-rs/src/services/hid.rs @@ -4,11 +4,6 @@ //! and circle pad information. It also provides information from the sound volume slider, //! the accelerometer, and the gyroscope. -use once_cell::sync::Lazy; -use std::sync::Mutex; - -use crate::services::ServiceReference; - bitflags::bitflags! { /// A set of flags corresponding to the button and directional pad /// inputs on the 3DS @@ -49,19 +44,12 @@ bitflags::bitflags! { /// when all instances of this struct fall out of scope. /// /// This service requires no special permissions to use. -#[non_exhaustive] -pub struct Hid { - _service_handler: ServiceReference, -} - -static HID_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); +pub struct Hid(()); /// Represents user input to the touchscreen. -#[non_exhaustive] pub struct TouchPosition(ctru_sys::touchPosition); /// Represents the current position of the 3DS circle pad. -#[non_exhaustive] pub struct CirclePosition(ctru_sys::circlePosition); /// Initializes the HID service. @@ -72,24 +60,15 @@ pub struct CirclePosition(ctru_sys::circlePosition); /// Since this service requires no special or elevated permissions, errors are /// rare in practice. impl Hid { - pub fn init() -> crate::Result { - let _service_handler = ServiceReference::new( - &HID_ACTIVE, - true, - || { - let r = unsafe { ctru_sys::hidInit() }; - if r < 0 { - return Err(r.into()); - } - - Ok(()) - }, - || unsafe { - ctru_sys::hidExit(); - }, - )?; - - Ok(Self { _service_handler }) + pub fn init() -> crate::Result { + unsafe { + let r = ctru_sys::hidInit(); + if r < 0 { + Err(r.into()) + } else { + Ok(Hid(())) + } + } } /// Scans the HID service for all user input occurring on the current @@ -169,15 +148,8 @@ impl CirclePosition { } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn hid_duplicate() { - // We don't need to build a `Hid` because the test runner has one already - let value = *HID_ACTIVE.lock().unwrap(); - - assert_eq!(value, 1); +impl Drop for Hid { + fn drop(&mut self) { + unsafe { ctru_sys::hidExit() }; } } diff --git a/ctru-rs/src/services/mod.rs b/ctru-rs/src/services/mod.rs index cc52685..0dc750c 100644 --- a/ctru-rs/src/services/mod.rs +++ b/ctru-rs/src/services/mod.rs @@ -3,12 +3,12 @@ pub mod fs; pub mod gspgpu; pub mod hid; pub mod ps; +mod reference; pub mod soc; pub mod sslc; -mod reference; pub use self::apt::Apt; pub use self::hid::Hid; pub use self::sslc::SslC; -pub (crate) use self::reference::ServiceReference; +pub(crate) use self::reference::ServiceReference; diff --git a/ctru-rs/src/services/reference.rs b/ctru-rs/src/services/reference.rs new file mode 100644 index 0000000..a2d1682 --- /dev/null +++ b/ctru-rs/src/services/reference.rs @@ -0,0 +1,44 @@ +use crate::Error; +use std::sync::Mutex; +pub(crate) struct ServiceReference { + counter: &'static Mutex, + close: Box, +} + +impl ServiceReference { + pub fn new( + counter: &'static Mutex, + allow_multiple: bool, + start: S, + close: E, + ) -> crate::Result + where + S: FnOnce() -> crate::Result<()>, + E: Fn() + 'static, + { + let mut value = counter.lock().unwrap(); // todo: handle poisoning + + if *value == 0 { + start()?; + } else if !allow_multiple { + return Err(Error::ServiceAlreadyActive); + } + + *value += 1; + + Ok(Self { + counter, + close: Box::new(close), + }) + } +} + +impl Drop for ServiceReference { + fn drop(&mut self) { + let mut value = self.counter.lock().unwrap(); // should probably handle poisoning - could just map_err to ignore it. + *value -= 1; + if *value == 0 { + (self.close)(); + } + } +} diff --git a/ctru-rs/src/services/sslc.rs b/ctru-rs/src/services/sslc.rs index 5c0d0e4..bdd5407 100644 --- a/ctru-rs/src/services/sslc.rs +++ b/ctru-rs/src/services/sslc.rs @@ -1,66 +1,35 @@ // TODO: Implement remaining functions -use once_cell::sync::Lazy; -use std::sync::Mutex; - -use crate::services::ServiceReference; - -#[non_exhaustive] -pub struct SslC { - _service_handler: ServiceReference, -} - -static SSLC_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); +pub struct SslC(()); impl SslC { /// Initialize sslc pub fn init() -> crate::Result { - let _service_handler = ServiceReference::new( - &SSLC_ACTIVE, - true, - || { - let r = unsafe { ctru_sys::sslcInit(0) }; - if r < 0 { - return Err(r.into()); - } - - Ok(()) - }, - || unsafe { - ctru_sys::sslcExit(); - }, - )?; - - Ok(Self { _service_handler }) + unsafe { + let r = ctru_sys::sslcInit(0); + if r < 0 { + Err(r.into()) + } else { + Ok(SslC(())) + } + } } /// Fill `buf` with `buf.len()` random bytes pub fn generate_random_data(&self, buf: &mut [u8]) -> crate::Result<()> { - let r = unsafe { ctru_sys::sslcGenerateRandomData(buf.as_ptr() as _, buf.len() as u32) }; - if r < 0 { - Err(r.into()) - } else { - Ok(()) + unsafe { + let r = ctru_sys::sslcGenerateRandomData(buf.as_ptr() as _, buf.len() as u32); + if r < 0 { + Err(r.into()) + } else { + Ok(()) + } } } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn sslc_duplicate() { - let _sslc = SslC::init().unwrap(); - - let value = *SSLC_ACTIVE.lock().unwrap(); - - assert_eq!(value, 1); - - drop(_sslc); - - let value = *SSLC_ACTIVE.lock().unwrap(); - - assert_eq!(value, 0); +impl Drop for SslC { + fn drop(&mut self) { + unsafe { ctru_sys::sslcExit() }; } } diff --git a/ctru-rs/src/srv.rs b/ctru-rs/src/srv.rs index 5ab83b6..7d2ad21 100644 --- a/ctru-rs/src/srv.rs +++ b/ctru-rs/src/srv.rs @@ -1,53 +1,20 @@ -use once_cell::sync::Lazy; -use std::sync::Mutex; - -use crate::services::ServiceReference; - -#[non_exhaustive] -pub struct Srv { - _service_handler: ServiceReference, -} - -static SRV_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); +pub struct Srv(()); impl Srv { - pub fn init() -> crate::Result { - let _service_handler = ServiceReference::new( - &SRV_ACTIVE, - true, - || { - let r = unsafe { ctru_sys::srvInit() }; - if r < 0 { - return Err(r.into()); - } - - Ok(()) - }, - || unsafe { - ctru_sys::srvExit(); - }, - )?; - - Ok(Self { _service_handler }) + pub fn init() -> crate::Result { + unsafe { + let r = ctru_sys::srvInit(); + if r < 0 { + Err(r.into()) + } else { + Ok(Srv(())) + } + } } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn srv_duplicate() { - let _srv = Srv::init().unwrap(); - - let value = *SRV_ACTIVE.lock().unwrap(); - - assert_eq!(value, 1); - - drop(_srv); - - let value = *SRV_ACTIVE.lock().unwrap(); - - assert_eq!(value, 0); +impl Drop for Srv { + fn drop(&mut self) { + unsafe { ctru_sys::srvExit() }; } } From f5234432eeef032ff3ac284cc7787d70a3172926 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Mon, 14 Mar 2022 14:23:46 +0100 Subject: [PATCH 7/9] Hid working normally in panic_hook --- ctru-rs/src/lib.rs | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/ctru-rs/src/lib.rs b/ctru-rs/src/lib.rs index bf3d247..14dc4d4 100644 --- a/ctru-rs/src/lib.rs +++ b/ctru-rs/src/lib.rs @@ -27,10 +27,11 @@ pub fn init() { } #[cfg(not(test))] - panic_hook_setup(); + _panic_hook_setup(); } -fn panic_hook_setup() { +fn _panic_hook_setup() { + use crate::services::hid::{Hid, KeyPad}; use std::panic::PanicInfo; let main_thread = thread::current().id(); @@ -44,30 +45,13 @@ fn panic_hook_setup() { if main_thread == thread::current().id() && console::Console::exists() { println!("\nPress SELECT to exit the software"); - // The use of unsafe functions here is basically obligatory. - // To have memory safety when using the `Hid` struct, we must not make more - // than one available at the same time, so no drop/service ownership issues arise. - // The problem here is that the `panic_hook` is run _before_ the app cleanup, - // so an `Hid` stuct may still be alive and thus make the `panic_hook` panic. - // If that were to happen, the system would have to reboot to properly close the app. - // - // Using `hidInit` is safe when another instance is open, and we can do safe operations afterwards. - // The only (probably) unsafe part of this system is the `hidExit`, since in a multithreaded - // environment some other threads may still be doing operations on the service - // before the cleanup, though the time window would be almost nonexistent, and it would only - // really be a problem in preemptive threads. - // - // TL;DR : This code is bad. - unsafe { - ctru_sys::hidInit(); - - loop { - ctru_sys::hidScanInput(); - let keys = services::hid::KeyPad::from_bits_truncate(ctru_sys::hidKeysDown()); - if keys.contains(services::hid::KeyPad::KEY_SELECT) { - ctru_sys::hidExit(); - break; - } + let hid = Hid::init().unwrap(); + + loop { + hid.scan_input(); + let keys = hid.keys_down(); + if keys.contains(KeyPad::KEY_SELECT) { + break; } } } From 1e03d8c6b756eb120de9090d7ee24d6f6cb179f7 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Mon, 14 Mar 2022 14:28:17 +0100 Subject: [PATCH 8/9] Fixed issues in normal builds --- ctru-rs/Cargo.toml | 2 +- ctru-rs/src/gfx.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ctru-rs/Cargo.toml b/ctru-rs/Cargo.toml index 53d39d3..baf2c79 100644 --- a/ctru-rs/Cargo.toml +++ b/ctru-rs/Cargo.toml @@ -19,6 +19,7 @@ pthread-3ds = { git = "https://github.com/Meziu/pthread-3ds.git" } libc = "0.2.116" bitflags = "1.0.0" widestring = "0.2.2" +once_cell = "1.10.0" [build-dependencies] toml = "0.5" @@ -29,7 +30,6 @@ futures = "0.3" time = "0.3.7" tokio = { version = "1.16", features = ["rt", "time", "sync", "macros"] } cfg-if = "1.0.0" -once_cell = "1.10.0" [features] default = ["romfs"] diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index f68f2bc..a516fe5 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -5,7 +5,7 @@ use std::cell::RefCell; use std::marker::PhantomData; use std::sync::Mutex; -use crate::error::{Error, Result}; +use crate::error::Result; use crate::services::gspgpu::{self, FramebufferFormat}; use crate::services::ServiceReference; From 0f781ce7d76e0c88014df8a945bbfff54a578b17 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Thu, 17 Mar 2022 14:08:18 +0100 Subject: [PATCH 9/9] Fixed nits and oversight in Soc --- ctru-rs/src/gfx.rs | 1 + ctru-rs/src/lib.rs | 22 ++++++++++++---------- ctru-rs/src/romfs.rs | 2 +- ctru-rs/src/services/reference.rs | 8 ++++---- ctru-rs/src/services/soc.rs | 12 +++--------- 5 files changed, 21 insertions(+), 24 deletions(-) diff --git a/ctru-rs/src/gfx.rs b/ctru-rs/src/gfx.rs index a516fe5..6400d97 100644 --- a/ctru-rs/src/gfx.rs +++ b/ctru-rs/src/gfx.rs @@ -220,6 +220,7 @@ impl From for ctru_sys::gfx3dSide_t { #[cfg(test)] mod tests { use super::*; + use crate::Error; #[test] fn gfx_duplicate() { diff --git a/ctru-rs/src/lib.rs b/ctru-rs/src/lib.rs index 22d0b44..810c5df 100644 --- a/ctru-rs/src/lib.rs +++ b/ctru-rs/src/lib.rs @@ -27,10 +27,11 @@ pub fn init() { } #[cfg(not(test))] - _panic_hook_setup(); + panic_hook_setup(); } -fn _panic_hook_setup() { +#[cfg(not(test))] +fn panic_hook_setup() { use crate::services::hid::{Hid, KeyPad}; use std::panic::PanicInfo; @@ -45,14 +46,15 @@ fn _panic_hook_setup() { if main_thread == std::thread::current().id() && console::Console::exists() { println!("\nPress SELECT to exit the software"); - let hid = Hid::init().unwrap(); - - loop { - hid.scan_input(); - let keys = hid.keys_down(); - if keys.contains(KeyPad::KEY_SELECT) { - break; - } + match Hid::init() { + Ok(hid) => loop { + hid.scan_input(); + let keys = hid.keys_down(); + if keys.contains(KeyPad::KEY_SELECT) { + break; + } + }, + Err(e) => println!("Error while intializing Hid controller during panic: {e}"), } } }); diff --git a/ctru-rs/src/romfs.rs b/ctru-rs/src/romfs.rs index dff45ca..e054985 100644 --- a/ctru-rs/src/romfs.rs +++ b/ctru-rs/src/romfs.rs @@ -52,7 +52,7 @@ mod tests { use super::*; #[test] - fn romfs_duplicate() { + fn romfs_counter() { let _romfs = RomFS::init().unwrap(); let value = *ROMFS_ACTIVE.lock().unwrap(); diff --git a/ctru-rs/src/services/reference.rs b/ctru-rs/src/services/reference.rs index a2d1682..7fac227 100644 --- a/ctru-rs/src/services/reference.rs +++ b/ctru-rs/src/services/reference.rs @@ -2,7 +2,7 @@ use crate::Error; use std::sync::Mutex; pub(crate) struct ServiceReference { counter: &'static Mutex, - close: Box, + close: Box, } impl ServiceReference { @@ -14,9 +14,9 @@ impl ServiceReference { ) -> crate::Result where S: FnOnce() -> crate::Result<()>, - E: Fn() + 'static, + E: Fn() + Send + Sync + 'static, { - let mut value = counter.lock().unwrap(); // todo: handle poisoning + let mut value = counter.lock().expect("Mutex Counter for ServiceReference is poisoned"); // todo: handle poisoning if *value == 0 { start()?; @@ -35,7 +35,7 @@ impl ServiceReference { impl Drop for ServiceReference { fn drop(&mut self) { - let mut value = self.counter.lock().unwrap(); // should probably handle poisoning - could just map_err to ignore it. + let mut value = self.counter.lock().expect("Mutex Counter for ServiceReference is poisoned"); // todo: handle poisoning *value -= 1; if *value == 0 { (self.close)(); diff --git a/ctru-rs/src/services/soc.rs b/ctru-rs/src/services/soc.rs index 5514adf..5705d06 100644 --- a/ctru-rs/src/services/soc.rs +++ b/ctru-rs/src/services/soc.rs @@ -33,7 +33,7 @@ impl Soc { pub fn init_with_buffer_size(num_bytes: usize) -> crate::Result { let _service_handler = ServiceReference::new( &SOC_ACTIVE, - true, + false, || { let soc_mem = unsafe { memalign(0x1000, num_bytes) } as *mut u32; let r = unsafe { ctru_sys::socInit(soc_mem, num_bytes as u32) }; @@ -65,18 +65,12 @@ impl Soc { #[cfg(test)] mod tests { use super::*; + use crate::Error; #[test] fn soc_duplicate() { let _soc = Soc::init().unwrap(); - let value = *SOC_ACTIVE.lock().unwrap(); - assert_eq!(value, 1); - - drop(_soc); - - let value = *SOC_ACTIVE.lock().unwrap(); - - assert_eq!(value, 0); + assert!(matches!(Soc::init(), Err(Error::ServiceAlreadyActive))) } }