Merge pull request #39 from corwinkuiper/logger-closure

implement callbacks with unboxed closure

mgba-test-runner/c/test-runner.c

@@ -10,20 +10,20 @@ void log_output(struct mLogger* _log, int category, enum mLogLevel level,
                 const char* format, va_list args);
 char* log_level_str(enum mLogLevel);
 
-static void (*EXTERNAL_LOGGING)(char*);
-
-struct mLogger LOGGER = {.log = log_output};
-
 struct MGBA {
+    struct mLogger mlogger;
     struct mCore* core;
     struct video_buffer videoBuffer;
     char* filename;
+    struct callback callback;
 };
 
 struct MGBA* new_runner(char* filename) {
     struct MGBA* mgba = malloc(sizeof(struct MGBA));
+    mgba->mlogger.log = log_output;
+    mgba->callback.callback = NULL;
 
-    mLogSetDefaultLogger(&LOGGER);
+    mLogSetDefaultLogger(&mgba->mlogger);
 
     char* filename_new = strdup(filename);
     mgba->filename = filename_new;
@@ -61,10 +61,13 @@ struct MGBA* new_runner(char* filename) {
     return mgba;
 }
 
-void set_logger(void logger(char*)) { EXTERNAL_LOGGING = logger; }
+void set_logger(struct MGBA* mgba, struct callback callback) {
+    mgba->callback = callback;
+}
 
 void free_runner(struct MGBA* mgba) {
     mgba->core->deinit(mgba->core);
+    mgba->callback.destroy(mgba->callback.data);
     free(mgba->filename);
     free(mgba->videoBuffer.buffer);
     free(mgba);
@@ -76,9 +79,12 @@ struct video_buffer get_video_buffer(struct MGBA* mgba) {
     return mgba->videoBuffer;
 }
 
-void log_output(struct mLogger* _log, int category, enum mLogLevel level,
+void log_output(struct mLogger* log, int category, enum mLogLevel level,
                 const char* format, va_list args) {
-    UNUSED(_log);
+    // cast log to mgba, this works as the logger is the top entry of the mgba
+    // struct
+    struct MGBA* mgba = (struct MGBA*)log;
+
     if (level & 31) {
         int32_t size = 0;
 
@@ -97,11 +103,11 @@ void log_output(struct mLogger* _log, int category, enum mLogLevel level,
         size -= offset;
         vsnprintf(&str[offset], size, format, args);
 
-        if (EXTERNAL_LOGGING != NULL) {
-            EXTERNAL_LOGGING(str);
-        } else {
+        if (mgba->callback.callback != NULL)
+            mgba->callback.callback(mgba->callback.data, str);
+        else
             printf("%s\n", str);
-        }
+
         free(str);
     }
 }

mgba-test-runner/c/test-runner.h

@@ -10,8 +10,14 @@ struct video_buffer {
     uint32_t* buffer;
 };
 
+struct callback {
+    void* data;
+    void (*callback)(void*, char[]);
+    void (*destroy)(void*);
+};
+
 struct MGBA* new_runner(char filename[]);
 void free_runner(struct MGBA* mgba);
-void set_logger(void logger(char[]));
+void set_logger(struct MGBA*, struct callback);
 void advance_frame(struct MGBA* mgba);
 struct video_buffer get_video_buffer(struct MGBA* mgba);

mgba-test-runner/src/main.rs

@@ -6,10 +6,8 @@ use image::io::Reader;
 use io::Write;
 use regex::Regex;
 use runner::VideoBuffer;
-use std::cell::RefCell;
 use std::io;
 use std::path::Path;
-use std::rc::Rc;
 
 #[derive(PartialEq, Eq, Debug, Clone)]
 enum Status {
@@ -19,14 +17,13 @@ enum Status {
 }
 
 fn test_file(file_to_run: &str) -> Status {
-    let finished = Rc::new(RefCell::new(Status::Running));
+    let mut finished = Status::Running;
     let debug_reader_mutex = Regex::new(r"^\[(.*)\] GBA Debug: (.*)$").unwrap();
 
     let mut mgba = runner::MGBA::new(file_to_run);
     let video_buffer = mgba.get_video_buffer();
 
-    let fin_closure = Rc::clone(&finished);
-    runner::set_logger(Box::new(move |message| {
+    mgba.set_logger(|message| {
         if let Some(captures) = debug_reader_mutex.captures(message) {
             let log_level = &captures[1];
             let out = &captures[2];
@@ -37,8 +34,7 @@ fn test_file(file_to_run: &str) -> Status {
                     Err(e) => {
                         println!("[failed]");
                         println!("{}", e);
-                        let mut done = fin_closure.borrow_mut();
-                        *done = Status::Failed;
+                        finished = Status::Failed;
                     }
                     Ok(_) => {}
                 }
@@ -50,28 +46,23 @@ fn test_file(file_to_run: &str) -> Status {
             }
 
             if log_level == "FATAL" {
-                let mut done = fin_closure.borrow_mut();
-                *done = Status::Failed;
+                finished = Status::Failed;
             }
 
             if out == "Tests finished successfully" {
-                let mut done = fin_closure.borrow_mut();
-                *done = Status::Sucess;
+                finished = Status::Sucess;
             }
         }
-    }));
+    });
 
     loop {
         mgba.advance_frame();
-        let done = finished.borrow();
-        if *done != Status::Running {
+        if finished != Status::Running {
             break;
         }
     }
 
-    runner::clear_logger();
-
-    return (*finished.borrow()).clone();
+    return finished;
 }
 
 fn main() -> Result<(), Error> {

mgba-test-runner/src/runner.rs

@@ -1,3 +1,4 @@
+use std::ffi::c_void;
 use std::ffi::CStr;
 use std::ffi::CString;
 
@@ -35,7 +36,6 @@ impl VideoBuffer {
 
 impl MGBA {
     pub fn new(filename: &str) -> Self {
-        unsafe { bindings::set_logger(Some(logger)) };
         let c_str = CString::new(filename).expect("should be able to make cstring from filename");
         MGBA {
             mgba: unsafe { bindings::new_runner(c_str.as_ptr() as *mut i8) },
@@ -54,28 +54,45 @@ impl MGBA {
     pub fn advance_frame(&mut self) {
         unsafe { bindings::advance_frame(self.mgba) }
     }
-}
-
-static mut CALLBACK: Option<Box<dyn Fn(&str)>> = None;
-
-pub fn set_logger(x: Box<dyn Fn(&str)>) {
-    unsafe {
-        assert!(CALLBACK.is_none());
-        CALLBACK = Some(x);
+    pub fn set_logger(&mut self, mut logger: impl FnMut(&str)) {
+        unsafe {
+            let callback = generate_c_callback(move |message: *mut i8| {
+                logger(
+                    CStr::from_ptr(message)
+                        .to_str()
+                        .expect("should be able to convert logging message to rust String"),
+                );
+            });
+            bindings::set_logger(self.mgba, callback)
+        }
     }
 }
 
-pub fn clear_logger() {
-    unsafe { CALLBACK = None }
+unsafe fn generate_c_callback<F>(f: F) -> bindings::callback
+where
+    F: FnMut(*mut i8),
+{
+    let data = Box::into_raw(Box::new(f));
+
+    bindings::callback {
+        callback: Some(call_closure::<F>),
+        data: data as *mut _,
+        destroy: Some(drop_box::<F>),
+    }
 }
 
-extern "C" fn logger(c_str: *mut i8) {
+extern "C" fn call_closure<F>(data: *mut c_void, message: *mut i8)
+where
+    F: FnMut(*mut i8),
+{
+    let callback_ptr = data as *mut F;
+    let callback = unsafe { &mut *callback_ptr };
+    callback(message);
+}
+
+extern "C" fn drop_box<T>(data: *mut c_void) {
     unsafe {
-        if let Some(f) = &CALLBACK {
-            f(CStr::from_ptr(c_str)
-                .to_str()
-                .expect("should be able to convert logging message to rust String"));
-        }
+        Box::from_raw(data as *mut T);
     }
 }