From 7f4e437c2307e3630eba165ba013c02028ef2abb Mon Sep 17 00:00:00 2001 From: ivmarkov Date: Sat, 22 Jun 2024 12:57:09 +0300 Subject: [PATCH] Address feedback from code review Co-authored-by: Andrei Litvin --- rs-matter/src/transport/network/btp/context.rs | 3 ++- rs-matter/src/transport/network/btp/session.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/rs-matter/src/transport/network/btp/context.rs b/rs-matter/src/transport/network/btp/context.rs index ae9e145b..fc8e16b5 100644 --- a/rs-matter/src/transport/network/btp/context.rs +++ b/rs-matter/src/transport/network/btp/context.rs @@ -292,7 +292,8 @@ where .find(|session| session.address() == address) { if !session.set_subscribed() { - unreachable!(); + warn!("Got a second subscribe request for an address which is already subscribed: {address}"); + Err(ErrorCode::InvalidState)?; } self.handshake_notif.notify(); diff --git a/rs-matter/src/transport/network/btp/session.rs b/rs-matter/src/transport/network/btp/session.rs index 18d9e36e..e09dd245 100644 --- a/rs-matter/src/transport/network/btp/session.rs +++ b/rs-matter/src/transport/network/btp/session.rs @@ -243,9 +243,12 @@ impl RecvWindow { } fn check_data_integrity(&self, hdr: &BtpHdr, payload: &[u8], mtu: u16) -> Result<(), Error> { - if hdr.is_handshake() // Handshake packets are not allowed here - || hdr.get_opcode().is_some() - { + if hdr.is_handshake() { + warn!("RX data integrity failure: Handshake packets are not allowed here"); + return Err(ErrorCode::InvalidData.into()); + } + + if hdr.get_opcode().is_some() { warn!("RX data integrity failure: Data and standalone ACK packets must not have an opcode"); return Err(ErrorCode::InvalidData.into()); } @@ -455,7 +458,7 @@ impl Session { /// (As per the Matter Core spec, at any moment in time, a session /// can send the BTP segments of a single BTP SDU, hence the need for locking.) /// - /// Will return `false` if the session is already locked for sending. + /// Will return `false` if sending is true and the session is already locked for sending. pub fn set_sending(&mut self, sending: bool) -> bool { if sending && self.send_window.sending { return false; // already in sending mode, cannot set twice