Modify process_event so that it does not mutate self, nor the buffer.

Better type annotations in the C SDK make it possible to not mutate
the seph_buffer, which can therefore be passed as a reference instead
of copying. In order for this to work, however, process_event need to
not mutate &self, which was necessary in order to call self.reply()
and update the buttons state (on X/S+ only).
By changing the return value to a Result, we can bubble up the error
instead; for the buttons, we can work on a temporary copy instead of
borrowing the entirety of &self mutably.

This avoids having to copy the io_buffer into the seph_buffer before
calling process_event.

This is technically a breaking change because process_event is
public. However, no app uses it.

Author: bigspider

ledger_device_sdk/src/io.rs

@@ -1,5 +1,7 @@
 #[cfg(any(target_os = "nanosplus", target_os = "nanox"))]
-use ledger_secure_sdk_sys::buttons::{get_button_event, ButtonEvent, ButtonsState};
+use ledger_secure_sdk_sys::buttons::{get_button_event, ButtonEvent};
+
+use ledger_secure_sdk_sys::buttons::ButtonsState;
 use ledger_secure_sdk_sys::seph as sys_seph;
 use ledger_secure_sdk_sys::*;
 
@@ -343,17 +345,25 @@ impl Comm {
         None
     }
 
-    pub fn process_event<T>(&mut self, mut seph_buffer: [u8; 272], length: i32) -> Option<Event<T>>
+    pub fn process_event<T>(
+        &self,
+        seph_buffer: &[u8],
+        length: i32,
+        _buttons_state: Option<&mut ButtonsState>, // only used on NanoX/NanoS+
+    ) -> Result<Option<Event<T>>, StatusWords>
     where
         T: TryFrom<ApduHeader>,
         Reply: From<<T as TryFrom<ApduHeader>>::Error>,
     {
+        if seph_buffer.len() < 3 {
+            panic!("Invalid SEPH buffer length");
+        }
+
         let tag = seph_buffer[0];
         let _len: usize = u16::from_be_bytes([seph_buffer[1], seph_buffer[2]]) as usize;
 
         if (length as usize) < _len + 3 {
-            self.reply(StatusWords::BadLen);
-            return None;
+            return Err(StatusWords::BadLen);
         }
 
         match seph::Events::from(tag) {
@@ -362,19 +372,21 @@ impl Comm {
             seph::Events::ButtonPushEvent => {
                 #[cfg(feature = "nano_nbgl")]
                 unsafe {
-                    ux_process_button_event(seph_buffer.as_mut_ptr());
+                    ux_process_button_event(seph_buffer.as_ptr());
                 }
-                let button_info = seph_buffer[3] >> 1;
-                if let Some(btn_evt) = get_button_event(&mut self.buttons, button_info) {
-                    return Some(Event::Button(btn_evt));
+                if let Some(buttons_state) = _buttons_state {
+                    let button_info = seph_buffer[3] >> 1;
+                    if let Some(btn_evt) = get_button_event(buttons_state, button_info) {
+                        return Ok(Some(Event::Button(btn_evt)));
+                    }
                 }
             }
 
             // SCREEN TOUCH EVENT
             #[cfg(any(target_os = "stax", target_os = "flex", target_os = "apex_p"))]
             seph::Events::ScreenTouchEvent => unsafe {
-                ux_process_finger_event(seph_buffer.as_mut_ptr());
-                return Some(Event::TouchEvent);
+                ux_process_finger_event(seph_buffer.as_ptr());
+                return Ok(Some(Event::TouchEvent));
             },
 
             // TICKER EVENT
@@ -388,7 +400,7 @@ impl Comm {
                 unsafe {
                     ux_process_ticker_event();
                 }
-                return Some(Event::Ticker);
+                return Ok(Some(Event::Ticker));
             }
 
             // ITC EVENT
@@ -435,9 +447,9 @@ impl Comm {
                         }
                     }
 
-                    _ => return None,
+                    _ => return Ok(None),
                 }
-                return None;
+                return Ok(None);
             }
 
             // DEFAULT EVENT
@@ -457,7 +469,7 @@ impl Comm {
                 }
             }
         }
-        None
+        Ok(None)
     }
 
     pub fn decode_event<T>(&mut self, length: i32) -> Option<Event<T>>
@@ -469,11 +481,30 @@ impl Comm {
 
         match seph::PacketTypes::from(packet_type) {
             seph::PacketTypes::PacketTypeSeph | seph::PacketTypes::PacketTypeSeEvent => {
+                // On devices with buttons, we make a copy of the current button state
+                #[cfg(any(target_os = "nanosplus", target_os = "nanox"))]
+                let mut buttons_copy = self.buttons;
+                #[cfg(any(target_os = "nanosplus", target_os = "nanox"))]
+                let buttons_state = Some(&mut buttons_copy);
+
+                #[cfg(not(any(target_os = "nanosplus", target_os = "nanox")))]
+                let buttons_state = None;
+
                 // SE or SEPH event
-                let mut seph_buffer = [0u8; 272];
-                seph_buffer[0..272].copy_from_slice(&self.io_buffer[1..273]);
-                if let Some(event) = self.process_event(seph_buffer, length - 1) {
-                    return Some(event);
+                let result = self.process_event(&self.io_buffer[1..273], length - 1, buttons_state);
+
+                // update buttons state, in case it was modified
+                #[cfg(any(target_os = "nanosplus", target_os = "nanox"))]
+                {
+                    self.buttons = buttons_copy;
+                }
+
+                match result {
+                    Ok(e) => return e,
+                    Err(status) => {
+                        self.reply(status);
+                        return None;
+                    }
                 }
             }
 

ledger_secure_sdk_sys/src/buttons.rs

@@ -1,6 +1,6 @@
 /// Structure keeping track of button pushes
 /// 1 -> left button, 2 -> right button
-#[derive(Default)]
+#[derive(Default, Debug, Copy, Clone)]
 pub struct ButtonsState {
     pub button_mask: u8,
     pub cmd_buffer: [u8; 4],
@@ -17,7 +17,7 @@ impl ButtonsState {
 
 /// Event types needed by
 /// an application
-#[derive(Eq, PartialEq)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
 pub enum ButtonEvent {
     LeftButtonPress,
     RightButtonPress,