From 4e8ad7616f4aa3f21aaeee29b5ac1d364b64704f Mon Sep 17 00:00:00 2001 From: modeco80 Date: Tue, 6 Aug 2024 07:50:24 -0400 Subject: [PATCH] retrovnc: cleanup and safety annotations --- README.md | 12 ++++++----- crates/retrovnc/src/egl.rs | 4 ++++ crates/retrovnc/src/main.rs | 40 +++++++++++++++++++++++++++++-------- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 10ad263..03ff4c7 100644 --- a/README.md +++ b/README.md @@ -1,14 +1,15 @@ # retrovnc -a headless Libretro frontend that exports a VNC server. +A fully headless Libretro frontend that exports a VNC server for display and input. -This is mostly a "fun project", and probably isn't a very great solution for remote gaming. +In layman terms, this lets you play games over VNC. Which isn't all that great, but hey. + +This is mostly a "fun project" and consists mostly of code I already wrote. # Dependencies - A C++ toolchain -- A Rust toolchain. -- Maybe libvncserver (i'm not sure, it seems like the package can build it). +- A Rust toolchain. # Building @@ -16,8 +17,9 @@ This is mostly a "fun project", and probably isn't a very great solution for rem # Usage -`$ retrovnc --core --rom ` +`$ retrovnc --core --rom ` (see `retrovnc --help` for more options) For disc-based titles it is probably a good idea to pass the cuesheet file. I will implement stuff later to make this less annoying. + diff --git a/crates/retrovnc/src/egl.rs b/crates/retrovnc/src/egl.rs index 3ae2e35..9d62d6f 100644 --- a/crates/retrovnc/src/egl.rs +++ b/crates/retrovnc/src/egl.rs @@ -1,3 +1,5 @@ +//! EGL bindings and helpers. + #[allow(non_camel_case_types)] #[allow(unused_imports)] mod egl_impl { @@ -15,6 +17,8 @@ mod egl_impl { include!(concat!(env!("OUT_DIR"), "/egl_bindings.rs")); + // TODO: Move these helpers to a new "helpers" module. + pub type GetPlatformDisplayExt = unsafe extern "C" fn( platform: types::EGLenum, native_display: *const std::ffi::c_void, diff --git a/crates/retrovnc/src/main.rs b/crates/retrovnc/src/main.rs index 0d85f1d..019f875 100644 --- a/crates/retrovnc/src/main.rs +++ b/crates/retrovnc/src/main.rs @@ -74,8 +74,12 @@ impl App { readback_buffer: Vec::new(), }); - // Very very nasty, but honestly it works. - // I'll look into cleaning it up later. + // SAFETY: The boxed allocation will never drop since the main loop always loops forever. + // Even if it did, the only way to touch the pointer involves the frontend library calling retro_run, + // and the core calling one of the given callbacks. Therefore this is gnarly, but "fine". + // + // I'm still not really sure how to tell the borrow checker that this is alright, + // short of Box::leak() (which I don't want to do, since ideally I'd like actual cleanup to occur). let obj = &mut *boxed as &mut dyn FrontendInterface; boxed.frontend = Some(Frontend::new(obj as *mut dyn FrontendInterface)); @@ -88,8 +92,13 @@ impl App { fn init(&mut self) { // Currently retrovnc just hardcodes the assumption of a single RetroPad. - let pad = &mut self.pad as *mut dyn InputDevice; + // SAFETY: This too won't ever be Use-After-Free'd because the only oppoturnity to + // goes away on drop as well. That's a bit flaky reasoning wise, but is true. + // + // In all honesty, I'm not sure this even needs to be a *mut so I could see if + // making it a immutable reference works. + let pad = &mut self.pad as *mut dyn InputDevice; self.get_frontend().plug_input_device(0, pad); // Initalize the display @@ -112,8 +121,8 @@ impl App { } fn load_core>(&mut self, path: P) -> Result<()> { + // Unload an existing core. if self.get_frontend().core_loaded() { - println!("???"); let _ = self.get_frontend().unload_core(); } @@ -194,6 +203,7 @@ impl App { self.egl_context = std::ptr::null(); } + /// Deletes all OpenGL FBO resources (the FBO itself, the render texture, and the renderbuffer used for depth) fn hw_gl_delete_fbo(&mut self) { unsafe { gl::DeleteFramebuffers(1, std::ptr::addr_of_mut!(self.fbo_id)); @@ -387,20 +397,35 @@ fn main() -> Result<()> { tracing::subscriber::set_global_default(subscriber).unwrap(); let matches = command!() - .arg(arg!(--core ).required(true)) + .about("Headless VNC libretro frontend (with GPU rendering support)") + .arg( + arg!(--core ) + .required(true) + .help("libretro core to load") + .short('c'), + ) // Not that it matters, but this is only really required for cores that require // content to be loaded; that's most cores, but libretro does support the difference. // TODO: A core will tell us if it requires content, if it's not provided we can yell and exit. - .arg(arg!(--rom ).required(false)) + .arg( + arg!(--rom ) + .required(false) + .help("ROM to load into core") + .short('r'), + ) .arg( arg!(--rfb_listen
) .required(false) + .help("VNC listen address") + .short('l') .default_value("127.0.0.1"), ) .arg( arg!(--rfb_port ) .value_parser(value_parser!(u16)) - .required(true), + .required(true) + .help("VNC listen port") + .short('p'), ) .get_matches(); @@ -419,7 +444,6 @@ fn main() -> Result<()> { listen_port: *matches.get_one::("rfb_port").unwrap(), }; - // Initalize the app let mut app = App::new(rfb_config)?; app.load_core(core_path)?;