Repository: PistonDevelopers/glfw-rs (language: rust)

Issue:
Crate leaks unsafe functionality into safe rust!!
i was writing my own wrapper and wondering how others solved wrapping monitors safely and i guess the answer is they simply dont...

there are several issues with just this screenshot where undefined behavior leaks into safe rust:
![Screenshot from 2024-01-30 01-56-39](https://github.com/PistonDevelopers/glfw-rs/assets/5054701/9902e677-f06b-4f59-aaf8-b2bbdb3c2854)

`glfwGetWindowMonitor()` might return null. this is not checked
`glfwGetMonitorPos()` it is not legal to supply null for the monitor paramter here (but might be given the function above)

and even further that monitor even saves a monitor pointer is illegal since `glfw*Events()` might invalidate those pointers making them point to trash memory. (after a monitor is disconnected)

and even further still there is no checking for the validity of the thread the monitor is created in. glfw is not thread safe for the most part and a monitor created on anything but the thread `glfwInit()` is called on is undefined behavior.


this is why i dont use any crates from crates.io. the state of rust is abysmal...


Discussion / hints:
![image](https://github.com/PistonDevelopers/glfw-rs/assets/44316547/65057faa-c7f2-42ae-a9f5-185d14523719)
Whoops! Looks like that was my fault. I'm going to fix this asap. Thanks for catching that!

Fix patch:
diff --git a/src/lib.rs b/src/lib.rs
index 08f884db..3dee445f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1142,6 +1142,26 @@ impl Glfw {
         }
     }
 
+    /// Supplies the window monitor to the closure provided, if it's fullscreen.
+    ///
+    /// # Example
+    ///
+    /// ~~~ignore
+    /// let (window, events) = glfw.with_window_monitor(|_, m| {
+    ///     glfw.create_window(300, 300, "Hello this is window",
+    ///         m.map_or(glfw::WindowMode::Windowed, |m| glfw::FullScreen(m)))
+    /// }).expect("Failed to create GLFW window.");
+    /// ~~~
+    pub fn with_window_monitor<T, F>(&mut self, window: &mut Window, f: F) -> T
+        where
+            F: FnOnce(&mut Self, Option<&mut Monitor>) -> T,
+    {
+        match unsafe { ffi::glfwGetWindowMonitor(window.ptr) } {
+            ptr if ptr.is_null() => f(self, None),
+            ptr => f(self, Some(&mut Monitor { ptr })),
+        }
+    }
+
     /// Supplies a vector of the currently connected monitors to the closure
     /// provided.
     ///
@@ -1156,18 +1176,20 @@ impl Glfw {
     /// ~~~
     pub fn with_connected_monitors<T, F>(&mut self, f: F) -> T
     where
-        F: FnOnce(&mut Self, &mut [Monitor]) -> T,
+        F: FnOnce(&mut Self, &[&mut Monitor]) -> T,
     {
         unsafe {
             let mut count = 0;
             let ptr = ffi::glfwGetMonitors(&mut count);
-            f(
-                self,
-                &mut slice::from_raw_parts(ptr as *const _, count as usize)
-                    .iter()
-                    .map(|&ptr| Monitor { ptr })
-                    .collect::<Vec<Monitor>>(),
-            )
+            let mut monitors = slice::from_raw_parts(ptr as *const _, count as usize)
+                .iter()
+                .map(|&ptr| Monitor { ptr })
+                .collect::<Vec<Monitor>>();
+
+            let refs: Vec<&mut Monitor> = monitors
+                .iter_mut()
+                .collect();
+            f(self, &refs)
         }
     }
 
@@ -1920,25 +1942,6 @@ impl std::fmt::Debug for Monitor {
 }
 
 impl Monitor {
-
-    /// Wrapper for `glfwGetPrimaryMonitor`.
-    pub fn from_primary() -> Self {
-        unsafe {
-            Self {
-                ptr: ffi::glfwGetPrimaryMonitor()
-            }
-        }
-    }
-
-    /// Wrapper for `glfwGetWindowMonitor`.
-    pub fn from_window(window: &Window) -> Self {
-        unsafe {
-            Self {
-                ptr: ffi::glfwGetWindowMonitor(window.ptr)
-            }
-        }
-    }
-
     /// Wrapper for `glfwGetMonitorPos`.
     pub fn get_pos(&self) -> (i32, i32) {
         unsafe {


Test patch: