refactor: write_padding! and read_padding! macros (#177)

Macros ensuring optimal handling of padding at compile time.

Cargo.lock

@@ -1750,7 +1750,7 @@ dependencies = [
 name = "ironrdp-cliprdr"
 version = "0.1.0"
 dependencies = [
- "bitflags 2.0.2",
+ "bitflags 2.3.3",
  "ironrdp-pdu",
 ]
 

crates/ironrdp-client/src/gui.rs

@@ -1,12 +1,12 @@
 use anyhow::Context as _;
 use softbuffer::GraphicsContext;
 use tokio::sync::mpsc;
+use winit::dpi::LogicalPosition;
 use winit::event::{self, Event, WindowEvent};
 use winit::event_loop::{ControlFlow, EventLoop, EventLoopBuilder};
 use winit::window::{Window, WindowBuilder};
 
 use crate::rdp::{RdpInputEvent, RdpOutputEvent};
-use winit::dpi::LogicalPosition;
 
 pub struct GuiContext {
     pub window: Window,

crates/ironrdp-cliprdr/src/pdu/capabilities.rs

@@ -1,11 +1,12 @@
-use crate::pdu::PartialHeader;
 use bitflags::bitflags;
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::{
-    cast_int, cast_length, ensure_fixed_part_size, ensure_size, invalid_message_err, padding, PduDecode, PduEncode,
-    PduResult,
+    cast_int, cast_length, ensure_fixed_part_size, ensure_size, invalid_message_err, read_padding, write_padding,
+    PduDecode, PduEncode, PduResult,
 };
 
+use crate::pdu::PartialHeader;
+
 /// Represents `CLIPRDR_CAPS`
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Capabilities {
@@ -29,7 +30,7 @@ impl PduEncode for Capabilities {
         ensure_size!(in: dst, size: self.inner_size());
 
         dst.write_u16(cast_length!(Self::NAME, "cCapabilitiesSets", self.capabilities.len())?);
-        padding::write(dst, 2);
+        write_padding!(dst, 2);
 
         for capability in &self.capabilities {
             capability.encode(dst)?;
@@ -53,7 +54,7 @@ impl<'de> PduDecode<'de> for Capabilities {
 
         ensure_fixed_part_size!(in: src);
         let capabilities_count = src.read_u16();
-        padding::read(src, 2);
+        read_padding!(src, 2);
 
         let mut capabilities = Vec::with_capacity(usize::from(capabilities_count));
 

crates/ironrdp-cliprdr/src/pdu/client_temporary_directory.rs

@@ -1,10 +1,12 @@
-use crate::pdu::PartialHeader;
+use std::borrow::Cow;
+
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::utils::{read_string_from_cursor, write_string_to_cursor, CharacterSet};
 use ironrdp_pdu::{
     cast_int, ensure_fixed_part_size, ensure_size, invalid_message_err, PduDecode, PduEncode, PduResult,
 };
-use std::borrow::Cow;
+
+use crate::pdu::PartialHeader;
 
 /// Represents `CLIPRDR_TEMP_DIRECTORY`
 #[derive(Debug, Clone, PartialEq, Eq)]

crates/ironrdp-cliprdr/src/pdu/file_contents.rs

@@ -1,9 +1,11 @@
-use crate::pdu::{ClipboardPduFlags, PartialHeader};
+use std::borrow::Cow;
+
 use bitflags::bitflags;
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::utils::{combine_u64, split_u64};
 use ironrdp_pdu::{cast_int, ensure_size, invalid_message_err, PduDecode, PduEncode, PduResult};
-use std::borrow::Cow;
+
+use crate::pdu::{ClipboardPduFlags, PartialHeader};
 
 bitflags! {
     /// Represents `dwFlags` field of `CLIPRDR_FILECONTENTS_REQUEST` structure.

crates/ironrdp-cliprdr/src/pdu/format_data/metafile.rs

@@ -1,7 +1,8 @@
+use std::borrow::Cow;
+
 use bitflags::bitflags;
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::{ensure_fixed_part_size, ensure_size, PduDecode, PduEncode, PduResult};
-use std::borrow::Cow;
 
 bitflags! {
     /// Represents `mappingMode` fields of `CLIPRDR_MFPICT` strucutre.

crates/ironrdp-cliprdr/src/pdu/format_data/mod.rs

@@ -2,15 +2,18 @@ mod file_list;
 mod metafile;
 mod palette;
 
-pub use file_list::*;
-pub use metafile::*;
-pub use palette::*;
+pub use self::file_list::*;
+pub use self::metafile::*;
+pub use self::palette::*;
+
+#[rustfmt::skip]
+use std::borrow::Cow;
 
-use crate::pdu::{ClipboardPduFlags, PartialHeader};
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::utils::{read_string_from_cursor, to_utf16_bytes, CharacterSet};
 use ironrdp_pdu::{cast_int, ensure_fixed_part_size, ensure_size, PduDecode, PduEncode, PduResult};
-use std::borrow::Cow;
+
+use crate::pdu::{ClipboardPduFlags, PartialHeader};
 
 /// Represents `CLIPRDR_FORMAT_DATA_RESPONSE`
 #[derive(Debug, Clone, PartialEq, Eq)]

crates/ironrdp-cliprdr/src/pdu/format_list.rs

@@ -1,8 +1,10 @@
-use crate::pdu::{ClipboardPduFlags, PartialHeader};
+use std::borrow::Cow;
+
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::utils::{read_string_from_cursor, to_utf16_bytes, write_string_to_cursor, CharacterSet};
 use ironrdp_pdu::{cast_int, ensure_size, invalid_message_err, PduDecode, PduEncode, PduResult};
-use std::borrow::Cow;
+
+use crate::pdu::{ClipboardPduFlags, PartialHeader};
 
 /// Represents `CLIPRDR_SHORT_FORMAT_NAME` and `CLIPRDR_LONG_FORMAT_NAME`
 #[derive(Debug, Clone, PartialEq, Eq)]

crates/ironrdp-cliprdr/src/pdu/lock.rs

@@ -1,7 +1,8 @@
-use crate::pdu::PartialHeader;
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::{cast_int, ensure_fixed_part_size, PduDecode, PduEncode, PduResult};
 
+use crate::pdu::PartialHeader;
+
 /// Represents `CLIPRDR_LOCK_CLIPDATA`/`CLIPRDR_UNLOCK_CLIPDATA`
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct LockDataId(pub u32);

crates/ironrdp-cliprdr/src/pdu/mod.rs

@@ -8,13 +8,14 @@ mod format_data;
 mod format_list;
 mod lock;
 
-pub use capabilities::*;
-pub use client_temporary_directory::*;
-pub use file_contents::*;
-pub use format_data::*;
-pub use format_list::*;
-pub use lock::*;
+pub use self::capabilities::*;
+pub use self::client_temporary_directory::*;
+pub use self::file_contents::*;
+pub use self::format_data::*;
+pub use self::format_list::*;
+pub use self::lock::*;
 
+#[rustfmt::skip]
 use bitflags::bitflags;
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::{ensure_fixed_part_size, invalid_message_err, PduDecode, PduEncode, PduResult};

crates/ironrdp-connector/src/legacy.rs

@@ -180,7 +180,9 @@ pub fn decode_share_data(ctx: SendDataIndicationCtx<'_>) -> ConnectorResult<Shar
     let ctx = decode_share_control(ctx)?;
 
     let rdp::headers::ShareControlPdu::Data(share_data_header) = ctx.pdu else {
-        return Err(general_err!("received unexpected Share Control Pdu (expected SHare Data Header)"));
+        return Err(general_err!(
+            "received unexpected Share Control Pdu (expected SHare Data Header)"
+        ));
     };
 
     Ok(ShareDataCtx {

crates/ironrdp-graphics/src/pointer.rs

@@ -19,13 +19,12 @@
 //! - andMask == 1, xorMask == 0(black color) -> Transparent pixel
 //! - andMask == 1, xorMask == 1(white color) -> Pixel is inverted
 
-use crate::color_conversion::rdp_16bit_to_rgb;
-use ironrdp_pdu::{
-    cursor::ReadCursor,
-    pointer::{ColorPointerAttribute, LargePointerAttribute, PointerAttribute},
-};
+use ironrdp_pdu::cursor::ReadCursor;
+use ironrdp_pdu::pointer::{ColorPointerAttribute, LargePointerAttribute, PointerAttribute};
 use thiserror::Error;
 
+use crate::color_conversion::rdp_16bit_to_rgb;
+
 #[derive(Debug, Error)]
 pub enum PointerError {
     #[error("Invalid pointer xorMask size. Expected: {expected}, actual: {actual}")]

crates/ironrdp-pdu/src/basic_output/fast_path/tests.rs

@@ -1,6 +1,7 @@
+use lazy_static::lazy_static;
+
 use super::*;
 use crate::{decode, encode};
-use lazy_static::lazy_static;
 
 const FAST_PATH_HEADER_WITH_SHORT_LEN_BUFFER: [u8; 2] = [0x80, 0x08];
 const FAST_PATH_HEADER_WITH_LONG_LEN_BUFFER: [u8; 3] = [0x80, 0x81, 0xE7];

crates/ironrdp-pdu/src/basic_output/pointer.rs

@@ -1,7 +1,5 @@
-use crate::{
-    cursor::{ReadCursor, WriteCursor},
-    PduDecode, PduEncode, PduResult,
-};
+use crate::cursor::{ReadCursor, WriteCursor};
+use crate::{PduDecode, PduEncode, PduResult};
 
 // Represents `TS_POINT16` described in [MS-RDPBCGR] 2.2.9.1.1.4.1
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]

crates/ironrdp-pdu/src/geometry.rs

@@ -263,10 +263,12 @@ impl<'de> PduDecode<'de> for ExclusiveRectangle {
 // Legacy code for serializing/deserializing exclusive rectangles as inclusive rectangles structure
 // TODO(@pacmancoder) this should be removed later
 mod legacy {
-    use super::*;
-    use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
     use std::io;
 
+    use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
+
+    use super::*;
+
     impl InclusiveRectangle {
         // TODO: clarify code related to rectangles (inclusive vs exclusive bounds, …)
         // See for instance:

crates/ironrdp-pdu/src/macros.rs

@@ -217,6 +217,41 @@ macro_rules! impl_pdu_borrowing {
     };
 }
 
+/// Writes zeroes using as few `write_u*` calls as possible.
+///
+/// This is similar to `ironrdp_pdu::padding::write`, but the loop is optimized out when a single
+/// operation is enough.
+#[macro_export]
+macro_rules! write_padding {
+    ($dst:expr, 1) => {
+        $dst.write_u8(0)
+    };
+    ($dst:expr, 2) => {
+        $dst.write_u16(0)
+    };
+    ($dst:expr, 4) => {
+        $dst.write_u32(0)
+    };
+    ($dst:expr, 8) => {
+        $dst.write_u64(0)
+    };
+    ($dst:expr, $n:expr) => {
+        $crate::padding::write($dst, $n)
+    };
+}
+
+/// Moves read cursor, ignoring padding bytes.
+///
+/// This is similar to `ironrdp_pdu::padding::read`, only exists for consistancy with `write_padding!`.
+#[macro_export]
+macro_rules! read_padding {
+    ($src:expr, $n:expr) => {
+        $crate::padding::read($src, $n)
+    };
+}
+
+// FIXME: legacy macros below
+
 #[macro_export]
 macro_rules! try_read_optional {
     ($e:expr, $ret:expr) => {

crates/ironrdp-pdu/src/mcs.rs

@@ -601,7 +601,7 @@ impl<'de> McsPdu<'de> for SendDataRequest<'de> {
 
         // dataPriority + segmentation
         ensure_size!(ctx: Self::MCS_NAME, in: src, size: 1);
-        crate::padding::read(src, 1);
+        read_padding!(src, 1);
 
         let (length, _) = per::read_length(src).map_err(per_field_err!("userDataLength"))?;
         let length = usize::from(length);
@@ -682,7 +682,7 @@ impl<'de> McsPdu<'de> for SendDataIndication<'de> {
 
         // dataPriority + segmentation
         ensure_size!(ctx: Self::MCS_NAME, in: src, size: 1);
-        crate::padding::read(src, 1);
+        read_padding!(src, 1);
 
         let (length, _) = per::read_length(src).map_err(per_field_err!("userDataLength"))?;
         let length = usize::from(length);

crates/ironrdp-pdu/src/nego.rs

@@ -225,8 +225,14 @@ impl<'de> X224Pdu<'de> for ConnectionRequest {
 
         let nego_data = NegoRequestData::read(src)?;
 
-        let Some(variable_part_rest_size) = variable_part_size.checked_sub(nego_data.as_ref().map(|data| data.size()).unwrap_or(0)) else {
-            return Err(PduError::invalid_message(Self::NAME, "TPDU header variable part", "advertised size too small"));
+        let Some(variable_part_rest_size) =
+            variable_part_size.checked_sub(nego_data.as_ref().map(|data| data.size()).unwrap_or(0))
+        else {
+            return Err(PduError::invalid_message(
+                Self::NAME,
+                "TPDU header variable part",
+                "advertised size too small",
+            ));
         };
 
         if variable_part_rest_size >= usize::from(Self::RDP_NEG_REQ_SIZE) {

crates/ironrdp-pdu/src/pcb.rs

@@ -55,7 +55,7 @@ impl<'de> PduDecode<'de> for PreconnectionBlob {
             ));
         }
 
-        crate::padding::read(src, 4); // flags
+        read_padding!(src, 4); // flags
 
         // The version field SHOULD be initialized by the client and SHOULD be ignored by the server,
         // as specified in sections 3.1.5.1 and 3.2.5.1.
@@ -118,7 +118,7 @@ impl PduEncode for PreconnectionBlob {
         ensure_size!(in: dst, size: pcb_size);
 
         dst.write_u32(cast_length!("cbSize", pcb_size)?); // cbSize
-        crate::padding::write(dst, 4); // flags
+        write_padding!(dst, 4); // flags
         dst.write_u32(self.version.0); // version
         dst.write_u32(self.id); // id
 

crates/ironrdp-pdu/src/tpdu.rs

@@ -138,10 +138,10 @@ impl TpduHeader {
         }
 
         if code == TpduCode::DATA {
-            crate::padding::read(src, 1); // EOT
+            read_padding!(src, 1); // EOT
         } else {
             ensure_size!(in: src, size: 5);
-            crate::padding::read(src, 5); // DST-REF, SRC-REF, Class 0
+            read_padding!(src, 5); // DST-REF, SRC-REF, Class 0
         }
 
         Ok(Self { li, code })