egui-winit: fix unsafe API of Clipboard::new (#2765)

* egui-winit: fix unsafe API of Clipboard::new

The old API allowed passing an arbitrary pointer. The new
API still breaks safety by allowing the object to outlive
the input, but is at least safer.

* Update crates/egui-winit/src/clipboard.rs

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

* Fix typo

* Update crates/egui-winit/src/clipboard.rs

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

* cargo fmt

* egui-winit: fix init_smithay_clipboard

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

crates/egui-winit/CHANGELOG.md

@@ -3,7 +3,7 @@ All notable changes to the `egui-winit` integration will be noted in this file.
 
 
 ## Unreleased
-
+* Fix unsafe API: remove `State::new_with_wayland_display`; change `Clipboard::new` to take `&EventLoopWindowTarget<T>`.
 
 ## 0.21.1 - 2023-02-12
 * Fixed crash when window position is in an invalid state, which could happen e.g. due to changes in monitor size or DPI ([#2722](https://github.com/emilk/egui/issues/2722)).

crates/egui-winit/src/clipboard.rs

@@ -1,4 +1,4 @@
-use std::os::raw::c_void;
+use winit::event_loop::EventLoopWindowTarget;
 
 /// Handles interfacing with the OS clipboard.
 ///
@@ -25,8 +25,12 @@ pub struct Clipboard {
 }
 
 impl Clipboard {
-    #[allow(unused_variables)]
+    /// Construct a new instance
-    pub fn new(#[allow(unused_variables)] wayland_display: Option<*mut c_void>) -> Self {
+    ///
+    /// # Safety
+    ///
+    /// The returned `Clipboard` must not outlive the input `_event_loop`.
+    pub fn new<T>(_event_loop: &EventLoopWindowTarget<T>) -> Self {
         Self {
             #[cfg(all(feature = "arboard", not(target_os = "android")))]
             arboard: init_arboard(),
@@ -41,7 +45,7 @@ impl Clipboard {
                 ),
                 feature = "smithay-clipboard"
             ))]
-            smithay: init_smithay_clipboard(wayland_display),
+            smithay: init_smithay_clipboard(_event_loop),
 
             clipboard: Default::default(),
         }
@@ -132,15 +136,28 @@ fn init_arboard() -> Option<arboard::Clipboard> {
     ),
     feature = "smithay-clipboard"
 ))]
-fn init_smithay_clipboard(
+fn init_smithay_clipboard<T>(
-    wayland_display: Option<*mut c_void>,
+    _event_loop: &EventLoopWindowTarget<T>,
 ) -> Option<smithay_clipboard::Clipboard> {
-    if let Some(display) = wayland_display {
+    // Note: ideally "smithay-clipboard" would imply "wayland", but it doesn't.
-        tracing::debug!("Initializing smithay clipboard…");
+    #[cfg(feature = "wayland")]
-        #[allow(unsafe_code)]
+    {
-        Some(unsafe { smithay_clipboard::Clipboard::new(display) })
+        use winit::platform::wayland::EventLoopWindowTargetExtWayland as _;
-    } else {
+        if let Some(display) = _event_loop.wayland_display() {
-        tracing::debug!("Cannot initialize smithay clipboard without a display handle");
+            tracing::debug!("Initializing smithay clipboard…");
+            #[allow(unsafe_code)]
+            Some(unsafe { smithay_clipboard::Clipboard::new(display) })
+        } else {
+            tracing::debug!("Cannot initialize smithay clipboard without a display handle");
+            None
+        }
+    }
+
+    #[cfg(not(feature = "wayland"))]
+    {
+        tracing::debug!(
+            "You need to enable the 'wayland' feature of 'egui-winit' to get a working clipboard"
+        );
         None
     }
 }

crates/egui-winit/src/lib.rs

@@ -9,8 +9,6 @@
 
 #![allow(clippy::manual_range_contains)]
 
-use std::os::raw::c_void;
-
 #[cfg(feature = "accesskit")]
 pub use accesskit_winit;
 pub use egui;
@@ -85,11 +83,12 @@ pub struct State {
 }
 
 impl State {
+    /// Construct a new instance
+    ///
+    /// # Safety
+    ///
+    /// The returned `State` must not outlive the input `_event_loop`.
     pub fn new<T>(event_loop: &EventLoopWindowTarget<T>) -> Self {
-        Self::new_with_wayland_display(wayland_display(event_loop))
-    }
-
-    pub fn new_with_wayland_display(wayland_display: Option<*mut c_void>) -> Self {
         let egui_input = egui::RawInput {
             has_focus: false, // winit will tell us when we have focus
             ..Default::default()
@@ -103,7 +102,7 @@ impl State {
             current_cursor_icon: None,
             current_pixels_per_point: 1.0,
 
-            clipboard: clipboard::Clipboard::new(wayland_display),
+            clipboard: clipboard::Clipboard::new(event_loop),
 
             simulate_touch_screen: false,
             pointer_touch_id: None,
@@ -871,28 +870,6 @@ fn translate_cursor(cursor_icon: egui::CursorIcon) -> Option<winit::window::Curs
     }
 }
 
-/// Returns a Wayland display handle if the target is running Wayland
-fn wayland_display<T>(_event_loop: &EventLoopWindowTarget<T>) -> Option<*mut c_void> {
-    #[cfg(feature = "wayland")]
-    #[cfg(any(
-        target_os = "linux",
-        target_os = "dragonfly",
-        target_os = "freebsd",
-        target_os = "netbsd",
-        target_os = "openbsd"
-    ))]
-    {
-        use winit::platform::wayland::EventLoopWindowTargetExtWayland as _;
-        return _event_loop.wayland_display();
-    }
-
-    #[allow(unreachable_code)]
-    {
-        let _ = _event_loop;
-        None
-    }
-}
-
 // ---------------------------------------------------------------------------
 
 /// Profiling macro for feature "puffin"