Change in simtrace2[master]: card_emu: Avoid recursive calls to card_set_state()

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/16602 )

Change subject: card_emu: Avoid recursive calls to card_set_state()
......................................................................

card_emu: Avoid recursive calls to card_set_state()

A function that is called to set the state should not in itself
contain logic to issue further state changes.

Let's shift the related block out to the end of card_emu_io_statechg(),
which is the only source of card_set_state() calls for the
WAIT_{POWER,CLK,RST} states anyway.

As an added benefit, the block of statements is now also executed if
there's no state change - something that was prevented by the
"if old == new" state guard at the top of card_set_state().  I believe
this may help us to cover more (non-standard) card activation sequences.

Change-Id: Ieefa1807099eb234cfd994bca83caaa0dcc919b6
---
M firmware/libcommon/source/card_emu.c
1 file changed, 17 insertions(+), 7 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c
index c3299de..34fac1b 100644
--- a/firmware/libcommon/source/card_emu.c
+++ b/firmware/libcommon/source/card_emu.c
@@ -426,13 +426,6 @@
 	case ISO_S_WAIT_RST:
 		/* disable Rx and Tx of UART */
 		card_emu_uart_enable(ch->uart_chan, 0);
-		/* check end activation state (only necessary if the reader to not respect the activation sequence) */
-		if (ch->vcc_active && ch->clocked && !ch->in_reset) {
-			/* enable the TC/ETU counter once reset has been released */
-			tc_etu_enable(ch->tc_chan);
-			/* prepare to send the ATR */
-			card_set_state(ch, ISO_S_WAIT_ATR);
-		}
 		break;
 	case ISO_S_WAIT_ATR:
 		/* Reset to initial Fi / Di ratio */
@@ -1111,6 +1104,23 @@
 		ch->in_reset = active;
 		break;
 	}
+
+	switch (ch->state) {
+	case ISO_S_WAIT_POWER:
+	case ISO_S_WAIT_CLK:
+	case ISO_S_WAIT_RST:
+		/* check end activation state (even if the reader does
+		 * not respect the activation sequence) */
+		if (ch->vcc_active && ch->clocked && !ch->in_reset) {
+			/* enable the TC/ETU counter once reset has been released */
+			tc_etu_enable(ch->tc_chan);
+			/* prepare to send the ATR */
+			card_set_state(ch, ISO_S_WAIT_ATR);
+		}
+		break;
+	default:
+		break;
+	}
 }
 
 /* User sets a new ATR to be returned during next card reset */

-- 
To view, visit https://gerrit.osmocom.org/c/simtrace2/+/16602
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: simtrace2
Gerrit-Branch: master
Gerrit-Change-Id: Ieefa1807099eb234cfd994bca83caaa0dcc919b6
Gerrit-Change-Number: 16602
Gerrit-PatchSet: 5
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191217/a701c7a6/attachment.htm>