Merge pull request #53 from rust-native-ui/fix-unsound-callbacks

Finish up callback unsoundness fixes

Author: NoraCodes

CHANGELOG.md

@@ -23,6 +23,8 @@ project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 * `LayoutGrid::insert_at` no longer takes `left` and `height` arguments
 * Many APIs which took `u64` or `i64` arguments now take `i32` for wider compatibility
 * The semi-unstable `iui::draw` subsystem is again exported to downstream consumers of the `iui` crate.
+* `UI::queue_main` and `UI::on_should_quit` now require passed closures to be `'static`, for soundness
+* All callback registration functions require that their callbacks live at least as long as the `UI` token, for soundness
 
 ### Deprecated
 

CONTRIBUTING.md renamed to CONTRIBUTING/CONTRIBUTING.md

@@ -34,7 +34,9 @@ from slowing down your PR.
 * Mark unsafe functions as `unsafe`. This includes _anything_ with potential
 undefined behavior, not just memory unsafety.
 * Document your code! This is the number one holdup for pull requests, especially
-large ones. Don't forget to insert appropriate entries into the changelog.
+large ones. This includes adding new concepts to the CONTRIBUTING directory (developer
+documentation).
+* Insert appropriate entries into the changelog.
 * If implementing a new feature from `ui`, please mention the stability of that
 feature in your pull request. We are fine with implementing unstable APIs from
 `ui`, but it's important to mark such APIs as unstable.

CONTRIBUTING/callbacks.md

@@ -0,0 +1,30 @@
+# Callbacks with libui
+
+libui provides a mechanism through which each callback can be passed arbitrary data. (An
+untyped buffer.)
+IUI uses this mechanism to provide a C "wrapper" function which converts any raw libui
+types into IUI Rust types, and does sanity checking.
+
+We do this with the functions `callback_helpers::to_heap_ptr`, which `Box`es up a Rust
+type and returns a pointer to the allocated memory, and `callback_helpers::from_void_ptr`,
+which reconstitutes the Rust type (specified using generics). These functions are `unsafe`
+for obvious reasons, and can also leak memory if used improperly.
+
+Their intended use is for `to_heap_ptr` to turn a Rust function into a bag of bits and for
+`from_void_ptr` to reconstitute that function into the _exact same type_, which we ensure
+by "generic locking" the user and wrapper functions, like so:
+
+```rust
+fn on_whatever<'ctx, F: FnMut(&Whatever) + 'static>(&mut self, _ctx: &'ctx UI, callback: F) {
+
+    fn c_callback<G: FnMut(&Whatever)> { /* ... do stuff ... */ }
+
+    ui_sys::uiWhateverOnWhatever(/* ... */, c_callback::<F>);
+}
+```
+
+This is somewhat verbose but ensures that the types do not deviate, which would be unsafe.
+
+Callbacks should be named `on_event` where `event` is, for instance, `clicked` or
+`closing`. The functions taken by callbacks must always have the `'static` bound.
+

iui/examples/basic.rs

@@ -1,13 +1,13 @@
 extern crate iui;
+use iui::controls::{Button, Group, Label, VerticalBox};
 use iui::prelude::*;
-use iui::controls::{Label, Button, VerticalBox, Group};
 
 fn main() {
     // Initialize the UI library
     let ui = UI::init().expect("Couldn't initialize UI library");
     // Create a window into which controls can be placed
     let mut win = Window::new(&ui, "Test App", 200, 200, WindowType::NoMenubar);
-    
+
     // Create a vertical layout to hold the controls
     let mut vbox = VerticalBox::new(&ui);
     vbox.set_padded(&ui, true);

iui/examples/canvas.rs

@@ -1,10 +1,8 @@
 extern crate iui;
 extern crate ui_sys;
 
-use iui::controls::{
-    Area, AreaDrawParams, AreaHandler, HorizontalBox, LayoutStrategy,
-};
-use iui::draw::{Brush, Path, SolidBrush, FillMode};
+use iui::controls::{Area, AreaDrawParams, AreaHandler, HorizontalBox, LayoutStrategy};
+use iui::draw::{Brush, FillMode, Path, SolidBrush};
 use iui::prelude::*;
 use std::f64::consts::PI;
 

iui/examples/files.rs

@@ -2,11 +2,11 @@
 //! and the Window::modal_err() call to display modal dialog boxes.
 
 extern crate iui;
+use iui::controls::{Button, MultilineEntry, VerticalBox};
 use iui::prelude::*;
-use iui::controls::{VerticalBox, MultilineEntry, Button};
-use std::io::prelude::*;
 use std::error::Error;
 use std::fs::File;
+use std::io::prelude::*;
 
 fn main() {
     // Initialize the UI
@@ -32,17 +32,38 @@ fn main() {
         move |_| {
             if let Some(path) = window.save_file(&ui) {
                 let mut file = match File::create(&path) {
-                    Err(why) => { window.modal_err(&ui, "I/O Error", &format!("Could not open file {}: {}", path.display(), why.description())); return; }
-                    Ok(f) => f
+                    Err(why) => {
+                        window.modal_err(
+                            &ui,
+                            "I/O Error",
+                            &format!(
+                                "Could not open file {}: {}",
+                                path.display(),
+                                why.description()
+                            ),
+                        );
+                        return;
+                    }
+                    Ok(f) => f,
                 };
                 match file.write_all(entry.value(&ui).as_bytes()) {
-                    Err(why) => { window.modal_err(&ui, "I/O Error", &format!("Could not write to file {}: {}", path.display(), why.description())); return; }
-                    Ok(_) => ()
+                    Err(why) => {
+                        window.modal_err(
+                            &ui,
+                            "I/O Error",
+                            &format!(
+                                "Could not write to file {}: {}",
+                                path.display(),
+                                why.description()
+                            ),
+                        );
+                        return;
+                    }
+                    Ok(_) => (),
                 };
-            }    
+            }
         }
     });
 
     ui.main();
-
 }