refactor!: enable unwrap_used clippy correctness lint (#965)

Co-authored-by: Benoît CORTIER <git.divisible626@passmail.com>

Author: RRRadicalEdward

Cargo.toml

@@ -102,7 +102,7 @@ float_cmp = "warn"
 lossy_float_literal = "warn"
 float_cmp_const = "warn"
 as_underscore = "warn"
-# TODO: unwrap_used = "warn" # Let’s either handle `None`, `Err` or use `expect` to give a reason.
+unwrap_used = "warn"
 large_stack_frames = "warn"
 mem_forget = "warn"
 mixed_read_write_in_expression = "warn"

benches/src/perfenc.rs

@@ -2,7 +2,7 @@
 #![allow(clippy::print_stderr)]
 #![allow(clippy::print_stdout)]
 
-use core::num::NonZero;
+use core::num::{NonZeroU16, NonZeroUsize};
 use core::time::Duration;
 use std::io::Write as _;
 use std::time::Instant;
@@ -59,13 +59,14 @@ async fn main() -> Result<(), anyhow::Error> {
         OptCodec::QoiZ => update_codecs.set_qoiz(Some(0)),
     };
 
-    let mut encoder = UpdateEncoder::new(DesktopSize { width, height }, flags, update_codecs);
+    let mut encoder = UpdateEncoder::new(DesktopSize { width, height }, flags, update_codecs)
+        .context("failed to initialize update encoder")?;
 
     let mut total_raw = 0u64;
     let mut total_enc = 0u64;
     let mut n_updates = 0u64;
     let mut updates = DisplayUpdates::new(file, DesktopSize { width, height }, fps);
-    while let Some(up) = updates.next_update().await {
+    while let Some(up) = updates.next_update().await? {
         if let DisplayUpdate::Bitmap(ref up) = up {
             total_raw += up.data.len() as u64;
         } else {
@@ -82,7 +83,7 @@ async fn main() -> Result<(), anyhow::Error> {
         }
         n_updates += 1;
         print!(".");
-        std::io::stdout().flush().unwrap();
+        std::io::stdout().flush()?;
     }
     println!();
 
@@ -119,20 +120,21 @@ impl DisplayUpdates {
 
 #[async_trait::async_trait]
 impl RdpServerDisplayUpdates for DisplayUpdates {
-    async fn next_update(&mut self) -> Option<DisplayUpdate> {
+    async fn next_update(&mut self) -> anyhow::Result<Option<DisplayUpdate>> {
         let stride = self.desktop_size.width as usize * 4;
         let frame_size = stride * self.desktop_size.height as usize;
         let mut buf = vec![0u8; frame_size];
-        if self.file.read_exact(&mut buf).await.is_err() {
-            return None;
-        }
+        // FIXME: AsyncReadExt::read_exact is not cancellation safe.
+        self.file.read_exact(&mut buf).await.context("read exact")?;
 
         let now = Instant::now();
         if let Some(last_update_time) = self.last_update_time {
             let elapsed = now - last_update_time;
             if self.fps > 0 && elapsed < Duration::from_millis(1000 / self.fps) {
                 sleep(Duration::from_millis(
-                    1000 / self.fps - u64::try_from(elapsed.as_millis()).unwrap(),
+                    1000 / self.fps
+                        - u64::try_from(elapsed.as_millis())
+                            .context("invalid `elapsed millis`: out of range integral conversion")?,
                 ))
                 .await;
             }
@@ -142,13 +144,13 @@ impl RdpServerDisplayUpdates for DisplayUpdates {
         let up = DisplayUpdate::Bitmap(BitmapUpdate {
             x: 0,
             y: 0,
-            width: self.desktop_size.width.try_into().unwrap(),
-            height: self.desktop_size.height.try_into().unwrap(),
+            width: NonZeroU16::new(self.desktop_size.width).context("width cannot be zero")?,
+            height: NonZeroU16::new(self.desktop_size.height).context("height cannot be zero")?,
             format: PixelFormat::RgbX32,
             data: buf.into(),
-            stride: NonZero::new(stride).unwrap(),
+            stride: NonZeroUsize::new(stride).context("stride cannot be zero")?,
         });
-        Some(up)
+        Ok(Some(up))
     }
 }
 

clippy.toml

@@ -3,3 +3,4 @@ semicolon-outside-block-ignore-multiline = true
 accept-comment-above-statement = true
 accept-comment-above-attributes = true
 allow-panic-in-tests = true
+allow-unwrap-in-tests = true

crates/ironrdp-acceptor/src/connection.rs

@@ -404,7 +404,7 @@ impl Sequence for Acceptor {
                     .into_iter()
                     .enumerate()
                     .map(|(i, channel)| {
-                        let channel_id = u16::try_from(i).unwrap() + self.io_channel_id + 1;
+                        let channel_id = u16::try_from(i).expect("always in the range") + self.io_channel_id + 1;
                         if let Some((type_id, c)) = channel {
                             self.static_channels.attach_channel_id(type_id, channel_id);
                             (channel_id, Some(c))

crates/ironrdp-ainput/src/lib.rs

@@ -6,8 +6,8 @@ use ironrdp_core::{
     ensure_fixed_part_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult, ReadCursor, WriteCursor,
 };
 use ironrdp_dvc::DvcEncode;
-use num_derive::{FromPrimitive, ToPrimitive};
-use num_traits::{FromPrimitive as _, ToPrimitive as _};
+use num_derive::FromPrimitive;
+use num_traits::FromPrimitive as _;
 // Advanced Input channel as defined from Freerdp, [here]:
 //
 // [here]: https://github.com/FreeRDP/FreeRDP/blob/master/include/freerdp/channels/ainput.h
@@ -93,11 +93,22 @@ impl<'de> Decode<'de> for VersionPdu {
     }
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive)]
+#[repr(u16)]
 pub enum ServerPduType {
     Version = 0x01,
 }
 
+impl ServerPduType {
+    #[expect(
+        clippy::as_conversions,
+        reason = "guarantees discriminant layout, and as is the only way to cast enum -> primitive"
+    )]
+    fn as_u16(&self) -> u16 {
+        *self as u16
+    }
+}
+
 impl<'a> From<&'a ServerPdu> for ServerPduType {
     fn from(s: &'a ServerPdu) -> Self {
         match s {
@@ -121,7 +132,7 @@ impl Encode for ServerPdu {
     fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
         ensure_fixed_part_size!(in: dst);
 
-        dst.write_u16(ServerPduType::from(self).to_u16().unwrap());
+        dst.write_u16(ServerPduType::from(self).as_u16());
         match self {
             ServerPdu::Version(pdu) => pdu.encode(dst),
         }
@@ -220,7 +231,7 @@ impl Encode for ClientPdu {
     fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
         ensure_fixed_part_size!(in: dst);
 
-        dst.write_u16(ClientPduType::from(self).to_u16().unwrap());
+        dst.write_u16(ClientPduType::from(self).as_u16());
         match self {
             ClientPdu::Mouse(pdu) => pdu.encode(dst),
         }
@@ -254,11 +265,22 @@ impl<'de> Decode<'de> for ClientPdu {
     }
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive)]
+#[repr(u16)]
 pub enum ClientPduType {
     Mouse = 0x02,
 }
 
+impl ClientPduType {
+    #[expect(
+        clippy::as_conversions,
+        reason = "guarantees discriminant layout, and as is the only way to cast enum -> primitive"
+    )]
+    fn as_u16(self) -> u16 {
+        self as u16
+    }
+}
+
 impl<'a> From<&'a ClientPdu> for ClientPduType {
     fn from(s: &'a ClientPdu) -> Self {
         match s {

crates/ironrdp-bench/benches/bench.rs

@@ -1,4 +1,4 @@
-use core::num::NonZero;
+use core::num::{NonZeroU16, NonZeroUsize};
 
 use criterion::{criterion_group, criterion_main, Criterion};
 use ironrdp_graphics::color_conversion::to_64x64_ycbcr_tile;
@@ -7,46 +7,70 @@ use ironrdp_server::bench::encoder::rfx::{rfx_enc, rfx_enc_tile};
 use ironrdp_server::BitmapUpdate;
 
 pub fn rfx_enc_tile_bench(c: &mut Criterion) {
+    const WIDTH: NonZeroU16 = NonZeroU16::new(64).expect("value is guaranteed to be non-zero");
+    const HEIGHT: NonZeroU16 = NonZeroU16::new(64).expect("value is guaranteed to be non-zero");
+    const STRIDE: NonZeroUsize = NonZeroUsize::new(64 * 4).expect("value is guaranteed to be non-zero");
+
     let quant = rfx::Quant::default();
     let algo = rfx::EntropyAlgorithm::Rlgr3;
+
     let bitmap = BitmapUpdate {
         x: 0,
         y: 0,
-        width: NonZero::new(64).unwrap(),
-        height: NonZero::new(64).unwrap(),
+        width: WIDTH,
+        height: HEIGHT,
         format: ironrdp_server::PixelFormat::ARgb32,
         data: vec![0; 64 * 64 * 4].into(),
-        stride: NonZero::new(64 * 4).unwrap(),
+        stride: STRIDE,
     };
     c.bench_function("rfx_enc_tile", |b| b.iter(|| rfx_enc_tile(&bitmap, &quant, algo, 0, 0)));
 }
 
 pub fn rfx_enc_bench(c: &mut Criterion) {
+    const WIDTH: NonZeroU16 = NonZeroU16::new(2048).expect("value is guaranteed to be non-zero");
+    const HEIGHT: NonZeroU16 = NonZeroU16::new(2048).expect("value is guaranteed to be non-zero");
+    // FIXME/QUESTION: It looks like we have a bug here, don't we? The stride value should be 2048 * 4.
+    const STRIDE: NonZeroUsize = NonZeroUsize::new(64 * 4).expect("value is guaranteed to be non-zero");
+
     let quant = rfx::Quant::default();
     let algo = rfx::EntropyAlgorithm::Rlgr3;
+
     let bitmap = BitmapUpdate {
         x: 0,
         y: 0,
-        width: NonZero::new(2048).unwrap(),
-        height: NonZero::new(2048).unwrap(),
+        width: WIDTH,
+        height: HEIGHT,
         format: ironrdp_server::PixelFormat::ARgb32,
         data: vec![0; 2048 * 2048 * 4].into(),
-        stride: NonZero::new(64 * 4).unwrap(),
+        stride: STRIDE,
     };
     c.bench_function("rfx_enc", |b| b.iter(|| rfx_enc(&bitmap, &quant, algo)));
 }
 
 pub fn to_ycbcr_bench(c: &mut Criterion) {
     const WIDTH: usize = 64;
     const HEIGHT: usize = 64;
+
     let input = vec![0; WIDTH * HEIGHT * 4];
     let stride = WIDTH * 4;
     let mut y = [0i16; WIDTH * HEIGHT];
     let mut cb = [0i16; WIDTH * HEIGHT];
     let mut cr = [0i16; WIDTH * HEIGHT];
     let format = ironrdp_graphics::image_processing::PixelFormat::ARgb32;
+
     c.bench_function("to_ycbcr", |b| {
-        b.iter(|| to_64x64_ycbcr_tile(&input, WIDTH, HEIGHT, stride, format, &mut y, &mut cb, &mut cr))
+        b.iter(|| {
+            to_64x64_ycbcr_tile(
+                &input,
+                WIDTH.try_into().expect("can't panic"),
+                HEIGHT.try_into().expect("can't panic"),
+                stride.try_into().expect("can't panic"),
+                format,
+                &mut y,
+                &mut cb,
+                &mut cr,
+            )
+        })
     });
 }
 

crates/ironrdp-blocking/src/connector.rs

@@ -100,7 +100,9 @@ fn resolve_generator(
     loop {
         match state {
             GeneratorState::Suspended(request) => {
-                let response = network_client.send(&request).unwrap();
+                let response = network_client.send(&request).map_err(|e| {
+                    ConnectorError::new("network client send", ironrdp_connector::ConnectorErrorKind::Credssp(e))
+                })?;
                 state = generator.resume(Ok(response));
             }
             GeneratorState::Completed(client_state) => {

crates/ironrdp-client/src/app.rs

@@ -5,9 +5,10 @@ use core::time::Duration;
 use std::sync::Arc;
 use std::time::Instant;
 
+use anyhow::Context as _;
 use raw_window_handle::{DisplayHandle, HasDisplayHandle as _};
 use tokio::sync::mpsc;
-use tracing::{debug, error, trace};
+use tracing::{debug, error, trace, warn};
 use winit::application::ApplicationHandler;
 use winit::dpi::{LogicalPosition, PhysicalSize};
 use winit::event::{self, WindowEvent};
@@ -38,7 +39,9 @@ impl App {
         // SAFETY: We drop the softbuffer context right before the event loop is stopped, thus making this safe.
         // FIXME: This is not a sufficient proof and the API is actually unsound as-is.
         let display_handle = unsafe {
-            core::mem::transmute::<DisplayHandle<'_>, DisplayHandle<'static>>(event_loop.display_handle().unwrap())
+            core::mem::transmute::<DisplayHandle<'_>, DisplayHandle<'static>>(
+                event_loop.display_handle().context("get display handle")?,
+            )
         };
         let context = softbuffer::Context::new(display_handle)
             .map_err(|e| anyhow::anyhow!("unable to initialize softbuffer context: {e}"))?;
@@ -65,9 +68,12 @@ impl App {
         };
         let scale_factor = (window.scale_factor() * 100.0) as u32;
 
+        let width = u16::try_from(size.width).expect("reasonable width");
+        let height = u16::try_from(size.height).expect("reasonable height");
+
         let _ = self.input_event_sender.send(RdpInputEvent::Resize {
-            width: u16::try_from(size.width).unwrap(),
-            height: u16::try_from(size.height).unwrap(),
+            width,
+            height,
             scale_factor,
             // TODO: it should be possible to get the physical size here, however winit doesn't make it straightforward.
             // FreeRDP does it based on DPI reading grabbed via [`SDL_GetDisplayDPI`](https://wiki.libsdl.org/SDL2/SDL_GetDisplayDPI):
@@ -160,7 +166,14 @@ impl ApplicationHandler<RdpOutputEvent> for App {
             // }
             WindowEvent::KeyboardInput { event, .. } => {
                 if let Some(scancode) = event.physical_key.to_scancode() {
-                    let scancode = ironrdp::input::Scancode::from_u16(u16::try_from(scancode).unwrap());
+                    let scancode = match u16::try_from(scancode) {
+                        Ok(scancode) => scancode,
+                        Err(_) => {
+                            warn!("Unsupported scancode: `{scancode:#X}`; ignored");
+                            return;
+                        }
+                    };
+                    let scancode = ironrdp::input::Scancode::from_u16(scancode);
 
                     let operation = match event.state {
                         event::ElementState::Pressed => ironrdp::input::Operation::KeyPressed(scancode),
@@ -325,13 +338,10 @@ impl ApplicationHandler<RdpOutputEvent> for App {
             RdpOutputEvent::Image { buffer, width, height } => {
                 trace!(width = ?width, height = ?height, "Received image with size");
                 trace!(window_physical_size = ?window.inner_size(), "Drawing image to the window with size");
-                self.buffer_size = (width, height);
+                self.buffer_size = (width.get(), height.get());
                 self.buffer = buffer;
                 surface
-                    .resize(
-                        NonZeroU32::new(u32::from(width)).unwrap(),
-                        NonZeroU32::new(u32::from(height)).unwrap(),
-                    )
+                    .resize(NonZeroU32::from(width), NonZeroU32::from(height))
                     .expect("surface resize");
 
                 window.request_redraw();

crates/ironrdp-client/src/config.rs

@@ -439,10 +439,9 @@ impl Config {
             desktop_scale_factor: 0, // Default to 0 per FreeRDP
             bitmap: Some(bitmap),
             client_build: semver::Version::parse(env!("CARGO_PKG_VERSION"))
-                .map(|version| version.major * 100 + version.minor * 10 + version.patch)
-                .unwrap_or(0)
+                .map_or(0, |version| version.major * 100 + version.minor * 10 + version.patch)
                 .pipe(u32::try_from)
-                .unwrap(),
+                .context("cargo package version")?,
             client_name: whoami::fallible::hostname().unwrap_or_else(|_| "ironrdp".to_owned()),
             // NOTE: hardcode this value like in freerdp
             // https://github.com/FreeRDP/FreeRDP/blob/4e24b966c86fdf494a782f0dfcfc43a057a2ea60/libfreerdp/core/settings.c#LL49C34-L49C70

crates/ironrdp-client/src/main.rs

@@ -22,8 +22,8 @@ fn main() -> anyhow::Result<()> {
     // TODO: get window size & scale factor from GUI/App
     let window_size = (1024, 768);
     config.connector.desktop_scale_factor = 0;
-    config.connector.desktop_size.width = u16::try_from(window_size.0).unwrap();
-    config.connector.desktop_size.height = u16::try_from(window_size.1).unwrap();
+    config.connector.desktop_size.width = window_size.0;
+    config.connector.desktop_size.height = window_size.1;
 
     let rt = runtime::Builder::new_multi_thread()
         .enable_all()