Merge pull request #50 from rust-windowing/better-unsafe

madsmtm · web-flow · commit 3b155497af1a · 2022-12-23T06:33:41.000+01:00
Safer `unsafe` usage
diff --git a/README.md b/README.md
@@ -55,7 +55,7 @@ For now, the priority for new platforms is:
 Example
 ==
 ```rust,no_run
-use Softbuffer::GraphicsContext;
+use softbuffer::GraphicsContext;
 use winit::event::{Event, WindowEvent};
 use winit::event_loop::{ControlFlow, EventLoop};
 use winit::window::WindowBuilder;
diff --git a/examples/animation.rs b/examples/animation.rs
@@ -1,8 +1,8 @@
 use instant::Instant;
 #[cfg(not(target_arch = "wasm32"))]
 use rayon::prelude::*;
-use std::f64::consts::PI;
 use softbuffer::GraphicsContext;
+use std::f64::consts::PI;
 use winit::event::{Event, WindowEvent};
 use winit::event_loop::{ControlFlow, EventLoop};
 use winit::window::WindowBuilder;
diff --git a/src/cg.rs b/src/cg.rs
@@ -23,22 +23,25 @@ impl CGImpl {
         let window = handle.ns_window as id;
         let view = handle.ns_view as id;
         let layer = CALayer::new();
-        let subview: id = NSView::alloc(nil).initWithFrame_(NSView::frame(view));
-        layer.set_contents_gravity(ContentsGravity::TopLeft);
-        layer.set_needs_display_on_bounds_change(false);
-        layer.set_contents_scale(window.backingScaleFactor());
-        subview.setLayer(layer.id());
-        subview.setAutoresizingMask_(NSViewWidthSizable | NSViewHeightSizable);
+        unsafe {
+            let subview: id = NSView::alloc(nil).initWithFrame_(NSView::frame(view));
+            layer.set_contents_gravity(ContentsGravity::TopLeft);
+            layer.set_needs_display_on_bounds_change(false);
+            layer.set_contents_scale(window.backingScaleFactor());
+            subview.setLayer(layer.id());
+            subview.setAutoresizingMask_(NSViewWidthSizable | NSViewHeightSizable);
 
-        view.addSubview_(subview); // retains subview (+1) = 2
-        let _: () = msg_send![subview, release]; // releases subview (-1) = 1
+            view.addSubview_(subview); // retains subview (+1) = 2
+            let _: () = msg_send![subview, release]; // releases subview (-1) = 1
+        }
         Ok(Self { layer })
     }
 
     pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) {
         let color_space = CGColorSpace::create_device_rgb();
         let data =
-            std::slice::from_raw_parts(buffer.as_ptr() as *const u8, buffer.len() * 4).to_vec();
+            unsafe { std::slice::from_raw_parts(buffer.as_ptr() as *const u8, buffer.len() * 4) }
+                .to_vec();
         let data_provider = CGDataProvider::from_buffer(Arc::new(data));
         let image = CGImage::new(
             width as usize,
@@ -52,6 +55,6 @@ impl CGImpl {
             false,
             kCGRenderingIntentDefault,
         );
-        self.layer.set_contents(image.as_ptr() as id);
+        unsafe { self.layer.set_contents(image.as_ptr() as id) };
     }
 }
diff --git a/src/lib.rs b/src/lib.rs
@@ -1,4 +1,5 @@
 #![doc = include_str!("../README.md")]
+#![deny(unsafe_op_in_unsafe_fn)]
 
 #[cfg(target_os = "macos")]
 #[macro_use]
@@ -56,7 +57,7 @@ macro_rules! make_dispatch {
                 match self {
                     $(
                         $(#[$attr])*
-                        Self::$name(inner) => inner.set_buffer(buffer, width, height),
+                        Self::$name(inner) => unsafe { inner.set_buffer(buffer, width, height) },
                     )*
                 }
             }
@@ -90,7 +91,7 @@ impl GraphicsContext {
         window: &W,
         display: &D,
     ) -> Result<Self, SwBufError> {
-        Self::from_raw(window.raw_window_handle(), display.raw_display_handle())
+        unsafe { Self::from_raw(window.raw_window_handle(), display.raw_display_handle()) }
     }
 
     /// Creates a new instance of this struct, using the provided raw window and display handles
@@ -108,22 +109,23 @@ impl GraphicsContext {
             (
                 RawWindowHandle::Xlib(xlib_window_handle),
                 RawDisplayHandle::Xlib(xlib_display_handle),
-            ) => Dispatch::X11(x11::X11Impl::new(xlib_window_handle, xlib_display_handle)?),
+            ) => Dispatch::X11(unsafe {
+                x11::X11Impl::new(xlib_window_handle, xlib_display_handle)?
+            }),
             #[cfg(all(feature = "wayland", any(target_os = "linux", target_os = "freebsd")))]
             (
                 RawWindowHandle::Wayland(wayland_window_handle),
                 RawDisplayHandle::Wayland(wayland_display_handle),
-            ) => Dispatch::Wayland(wayland::WaylandImpl::new(
-                wayland_window_handle,
-                wayland_display_handle,
-            )?),
+            ) => Dispatch::Wayland(unsafe {
+                wayland::WaylandImpl::new(wayland_window_handle, wayland_display_handle)?
+            }),
             #[cfg(target_os = "windows")]
             (RawWindowHandle::Win32(win32_handle), _) => {
-                Dispatch::Win32(win32::Win32Impl::new(&win32_handle)?)
+                Dispatch::Win32(unsafe { win32::Win32Impl::new(&win32_handle)? })
             }
             #[cfg(target_os = "macos")]
             (RawWindowHandle::AppKit(appkit_handle), _) => {
-                Dispatch::CG(cg::CGImpl::new(appkit_handle)?)
+                Dispatch::CG(unsafe { cg::CGImpl::new(appkit_handle)? })
             }
             #[cfg(target_arch = "wasm32")]
             (RawWindowHandle::Web(web_handle), _) => Dispatch::Web(web::WebImpl::new(web_handle)?),
diff --git a/src/orbital.rs b/src/orbital.rs
@@ -15,15 +15,17 @@ impl OrbitalMap {
         let size = pages * syscall::PAGE_SIZE;
 
         // Map window buffer
-        let address = syscall::fmap(
-            fd,
-            &syscall::Map {
-                offset: 0,
-                size,
-                flags: syscall::PROT_READ | syscall::PROT_WRITE,
-                address: 0,
-            },
-        )?;
+        let address = unsafe {
+            syscall::fmap(
+                fd,
+                &syscall::Map {
+                    offset: 0,
+                    size,
+                    flags: syscall::PROT_READ | syscall::PROT_WRITE,
+                    address: 0,
+                },
+            )?
+        };
 
         Ok(Self { address, size })
     }
@@ -69,14 +71,17 @@ impl OrbitalImpl {
 
         {
             // Map window buffer
-            let window_map = OrbitalMap::new(window_fd, window_width * window_height * 4)
-                .expect("failed to map orbital window");
+            let window_map =
+                unsafe { OrbitalMap::new(window_fd, window_width * window_height * 4) }
+                    .expect("failed to map orbital window");
 
             // Window buffer is u32 color data in 0xAABBGGRR format
-            let window_data = slice::from_raw_parts_mut(
-                window_map.address as *mut u32,
-                window_width * window_height,
-            );
+            let window_data = unsafe {
+                slice::from_raw_parts_mut(
+                    window_map.address as *mut u32,
+                    window_width * window_height,
+                )
+            };
 
             // Copy each line, cropping to fit
             let width = width_u16 as usize;
diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs
@@ -27,9 +27,9 @@ impl WaylandImpl {
         window_handle: WaylandWindowHandle,
         display_handle: WaylandDisplayHandle,
     ) -> Result<Self, SwBufError> {
-        let conn = Connection::from_backend(Backend::from_foreign_display(
-            display_handle.display as *mut _,
-        ));
+        // SAFETY: Ensured by user
+        let backend = unsafe { Backend::from_foreign_display(display_handle.display as *mut _) };
+        let conn = Connection::from_backend(backend);
         let (globals, event_queue) = unwrap(
             registry_queue_init(&conn),
             "Failed to make round trip to server",
@@ -40,10 +40,12 @@ impl WaylandImpl {
             "Failed to instantiate Wayland Shm",
         )?;
         let surface_id = unwrap(
-            ObjectId::from_ptr(
-                wl_surface::WlSurface::interface(),
-                window_handle.surface as _,
-            ),
+            unsafe {
+                ObjectId::from_ptr(
+                    wl_surface::WlSurface::interface(),
+                    window_handle.surface as _,
+                )
+            },
             "Failed to create proxy for surface ID.",
         )?;
         let surface = unwrap(
diff --git a/src/win32.rs b/src/win32.rs
@@ -46,7 +46,7 @@ impl Win32Impl {
         // Get the handle to the device context.
         // SAFETY: We have confirmed that the window handle is valid.
         let hwnd = handle.hwnd as HWND;
-        let dc = GetDC(hwnd);
+        let dc = unsafe { GetDC(hwnd) };
 
         // GetDC returns null if there is a platform error.
         if dc == 0 {
@@ -61,39 +61,67 @@ impl Win32Impl {
 
     pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) {
         // Create a new bitmap info struct.
-        let mut bitmap_info: BitmapInfo = mem::zeroed();
-
-        bitmap_info.bmi_header.biSize = mem::size_of::<BITMAPINFOHEADER>() as u32;
-        bitmap_info.bmi_header.biPlanes = 1;
-        bitmap_info.bmi_header.biBitCount = 32;
-        bitmap_info.bmi_header.biCompression = BI_BITFIELDS;
-        bitmap_info.bmi_header.biWidth = width as i32;
-        bitmap_info.bmi_header.biHeight = -(height as i32);
-        bitmap_info.bmi_colors[0].rgbRed = 0xff;
-        bitmap_info.bmi_colors[1].rgbGreen = 0xff;
-        bitmap_info.bmi_colors[2].rgbBlue = 0xff;
+        let bmi_header = BITMAPINFOHEADER {
+            biSize: mem::size_of::<BITMAPINFOHEADER>() as u32,
+            biWidth: width as i32,
+            biHeight: -(height as i32),
+            biPlanes: 1,
+            biBitCount: 32,
+            biCompression: BI_BITFIELDS,
+            biSizeImage: 0,
+            biXPelsPerMeter: 0,
+            biYPelsPerMeter: 0,
+            biClrUsed: 0,
+            biClrImportant: 0,
+        };
+        let zero_quad = RGBQUAD {
+            rgbBlue: 0,
+            rgbGreen: 0,
+            rgbRed: 0,
+            rgbReserved: 0,
+        };
+        let bmi_colors = [
+            RGBQUAD {
+                rgbRed: 0xff,
+                ..zero_quad
+            },
+            RGBQUAD {
+                rgbGreen: 0xff,
+                ..zero_quad
+            },
+            RGBQUAD {
+                rgbBlue: 0xff,
+                ..zero_quad
+            },
+        ];
+        let bitmap_info = BitmapInfo {
+            bmi_header,
+            bmi_colors,
+        };
 
         // Stretch the bitmap onto the window.
         // SAFETY:
         //  - The bitmap information is valid.
         //  - The buffer is a valid pointer to image data.
-        StretchDIBits(
-            self.dc,
-            0,
-            0,
-            width as c_int,
-            height as c_int,
-            0,
-            0,
-            width as c_int,
-            height as c_int,
-            buffer.as_ptr().cast(),
-            &bitmap_info as *const BitmapInfo as *const _,
-            DIB_RGB_COLORS,
-            SRCCOPY,
-        );
+        unsafe {
+            StretchDIBits(
+                self.dc,
+                0,
+                0,
+                width as c_int,
+                height as c_int,
+                0,
+                0,
+                width as c_int,
+                height as c_int,
+                buffer.as_ptr().cast(),
+                &bitmap_info as *const BitmapInfo as *const _,
+                DIB_RGB_COLORS,
+                SRCCOPY,
+            )
+        };
 
         // Validate the window.
-        ValidateRect(self.window, std::ptr::null_mut());
+        unsafe { ValidateRect(self.window, std::ptr::null_mut()) };
     }
 }
diff --git a/src/x11.rs b/src/x11.rs
@@ -65,14 +65,15 @@ impl X11Impl {
         // it could mean either screen index zero, or that the screen number was not set. We
         // can't tell which, so we'll just assume that the screen number was not set.
         let screen = match display_handle.screen {
-            0 => (lib.XDefaultScreen)(display_handle.display as *mut Display),
+            0 => unsafe { (lib.XDefaultScreen)(display_handle.display as *mut Display) },
             screen => screen,
         };
 
         // Use the default graphics context, visual and depth for this screen.
-        let gc = (lib.XDefaultGC)(display_handle.display as *mut Display, screen);
-        let visual = (lib.XDefaultVisual)(display_handle.display as *mut Display, screen);
-        let depth = (lib.XDefaultDepth)(display_handle.display as *mut Display, screen);
+        let gc = unsafe { (lib.XDefaultGC)(display_handle.display as *mut Display, screen) };
+        let visual =
+            unsafe { (lib.XDefaultVisual)(display_handle.display as *mut Display, screen) };
+        let depth = unsafe { (lib.XDefaultDepth)(display_handle.display as *mut Display, screen) };
 
         Ok(Self {
             window_handle,
@@ -86,35 +87,39 @@ impl X11Impl {
 
     pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) {
         // Create the image from the buffer.
-        let image = (self.lib.XCreateImage)(
-            self.display_handle.display as *mut Display,
-            self.visual,
-            self.depth as u32,
-            ZPixmap,
-            0,
-            (buffer.as_ptr()) as *mut c_char,
-            width as u32,
-            height as u32,
-            32,
-            (width * 4) as i32,
-        );
+        let image = unsafe {
+            (self.lib.XCreateImage)(
+                self.display_handle.display as *mut Display,
+                self.visual,
+                self.depth as u32,
+                ZPixmap,
+                0,
+                (buffer.as_ptr()) as *mut c_char,
+                width as u32,
+                height as u32,
+                32,
+                (width * 4) as i32,
+            )
+        };
 
         // Draw the image to the window.
-        (self.lib.XPutImage)(
-            self.display_handle.display as *mut Display,
-            self.window_handle.window,
-            self.gc,
-            image,
-            0,
-            0,
-            0,
-            0,
-            width as c_uint,
-            height as c_uint,
-        );
+        unsafe {
+            (self.lib.XPutImage)(
+                self.display_handle.display as *mut Display,
+                self.window_handle.window,
+                self.gc,
+                image,
+                0,
+                0,
+                0,
+                0,
+                width as c_uint,
+                height as c_uint,
+            )
+        };
 
         // Delete the image data.
-        (*image).data = std::ptr::null_mut();
-        (self.lib.XDestroyImage)(image);
+        unsafe { (*image).data = std::ptr::null_mut() };
+        unsafe { (self.lib.XDestroyImage)(image) };
     }
 }
