lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43129?usp=email )
(
1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: firmware: sniffer: fix ~INS procedure byte comparison ......................................................................
firmware: sniffer: fix ~INS procedure byte comparison
(~g_tpdu.packet[1]) == byte can never be true. Unary ~ applies the integer promotions first, so for tpdu INS = 0xA4 lhs should be 0x5B but as int it gets zero extended to at least 16 bits and then flipped, so it is 0xFFFFFF5B = -165, byte promotes to 0..255.
The ack was therefore dead code -> fallthrough to SW1 branch, fails 0x6x/0x9x test, TPDU gets flagged SNIFF_DATA_FLAG_ERROR_MALFORMED from what I can tell. But I am losing track of all these arcane issues to be honest.
Narrow the complement back to 8 bits. Fyi this is unrelated to signedness and not specific to ARM.
Change-Id: I800f50ef35356429d07aa685ea919e70ec34946e --- M firmware/libcommon/source/sniffer.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve lynxis lazus: Looks good to me, approved
diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 948eee8..419e8fb 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -775,7 +775,7 @@ } else if (g_tpdu.packet[1] == byte) { /* get all remaining data bytes */ change_tpdu_state(TPDU_S_DATA_REMAINING); break; - } else if ((~g_tpdu.packet[1]) == byte) { /* get single data byte */ + } else if ((uint8_t)(~g_tpdu.packet[1]) == byte) { /* get single data byte */ change_tpdu_state(TPDU_S_DATA_SINGLE); break; }