retrovnc: cleanup and safety annotations

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 <CORE> --rom <ROM>`
+`$ retrovnc --core <CORE> --rom <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.
 
 
+

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,

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<P: AsRef<Path>>(&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 <VALUE>).required(true))
+		.about("Headless VNC libretro frontend (with GPU rendering support)")
+		.arg(
+			arg!(--core <VALUE>)
+				.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 <VALUE>).required(false))
+		.arg(
+			arg!(--rom <VALUE>)
+				.required(false)
+				.help("ROM to load into core")
+				.short('r'),
+		)
 		.arg(
 			arg!(--rfb_listen <ADDRESS>)
 				.required(false)
+				.help("VNC listen address")
+				.short('l')
 				.default_value("127.0.0.1"),
 		)
 		.arg(
 			arg!(--rfb_port <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::<u16>("rfb_port").unwrap(),
 	};
 
-	// Initalize the app
 	let mut app = App::new(rfb_config)?;
 
 	app.load_core(core_path)?;