laforge has submitted this change. (
https://gerrit.osmocom.org/c/pysim/+/34932?usp=email )
Change subject: runtime: refactor file selection methods select and select_file
......................................................................
runtime: refactor file selection methods select and select_file
The implementation of the methods select and select_file of class
RuntimeLchan is a bit complex. We access the card directly in several
places which makes it difficult to track the state changes. We should
clean this up so that we call self.rs.card.select_adf_by_aid/
self.scc.select_file from a single place only.
This means that the method select uses the method select_file. This
results in a much cleaner implementation. We also should take care
that the important states that we track (selected_file, selected_adf,
etc.) are updated by a single private method. Since the update always
must happen after a select _select_post is a good place to do this.
Related: OS#5418
Change-Id: I9ae213f3b078983f3e6d4c11db38fdbe504c84f2
---
M pySim/runtime.py
1 file changed, 72 insertions(+), 40 deletions(-)
Approvals:
fixeria: Looks good to me, but someone else must approve
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/pySim/runtime.py b/pySim/runtime.py
index 1155433..f836ec8 100644
--- a/pySim/runtime.py
+++ b/pySim/runtime.py
@@ -255,6 +255,8 @@
raise ValueError(
"Cannot select unknown file by name %s, only hexadecimal 4
digit FID is allowed" % fid)
+ self._select_pre(cmd_app)
+
try:
# We access the card through the select_file method of the scc
object.
# If we succeed, we know that the file exists on the card and we
may
@@ -264,6 +266,7 @@
# so we do not have to update any state in that case.
(data, sw) = self.scc.select_file(fid)
except SwMatchError as swm:
+ self._select_post(cmd_app)
k = self.interpret_sw(swm.sw_actual)
if not k:
raise(swm)
@@ -282,8 +285,8 @@
desc="elementary file, manually added at
runtime")
self.selected_file.add_files([f])
- self.selected_file = f
- return select_resp, data
+
+ self._select_post(cmd_app, f, data)
def _select_pre(self, cmd_app):
# unregister commands of old file
@@ -291,7 +294,17 @@
for c in self.selected_file.shell_commands:
cmd_app.unregister_command_set(c)
- def _select_post(self, cmd_app):
+ def _select_post(self, cmd_app, file:Optional[CardFile] = None,
select_resp_data = None):
+ # we store some reference data (see above) about the currently
selected file.
+ # This data must be updated after every select.
+ if file:
+ self.selected_file = file
+ if isinstance(file, CardADF):
+ self.selected_adf = file
+ if select_resp_data:
+ self.selected_file_fcp_hex = select_resp_data
+ self.selected_file_fcp =
self.selected_file.decode_select_response(select_resp_data)
+
# register commands of new file
if cmd_app and self.selected_file.shell_commands:
for c in self.selected_file.shell_commands:
@@ -308,22 +321,31 @@
inter_path = self.selected_file.build_select_path_to(file)
if not inter_path:
raise RuntimeError('Cannot determine path from %s to %s' %
(self.selected_file, file))
-
self._select_pre(cmd_app)
- for p in inter_path:
+ # be sure the variables that we pass to _select_post contain valid
values.
+ selected_file = self.selected_file
+ data = self.selected_file_fcp_hex
+
+ for f in inter_path:
try:
- if isinstance(p, CardADF):
- (data, sw) = self.rs.card.select_adf_by_aid(p.aid,
scc=self.scc)
- self.selected_adf = p
+ # We now directly accessing the card to perform the selection.
This
+ # will change the state of the card, so we must take care to
update
+ # the local state (lchan) as well. This is done in the method
+ # _select_post. It should be noted that the caller must always
use
+ # the methods select_file or select. The caller must not
access the
+ # card directly since this would lead into an incoherence of
the
+ # card state and the state of the lchan.
+ if isinstance(f, CardADF):
+ (data, sw) = self.rs.card.select_adf_by_aid(f.aid,
scc=self.scc)
else:
- (data, sw) = self.scc.select_file(p.fid)
- self.selected_file = p
+ (data, sw) = self.scc.select_file(f.fid)
+ selected_file = f
except SwMatchError as swm:
- self._select_post(cmd_app)
+ self._select_post(cmd_app, selected_file, data)
raise(swm)
- self._select_post(cmd_app)
+ self._select_post(cmd_app, f, data)
def select(self, name: str, cmd_app=None):
"""Select a file (EF, DF, ADF, MF, ...).
@@ -332,9 +354,11 @@
name : Name of file to select
cmd_app : Command Application State (for unregistering old file
commands)
"""
+ # if any intermediate step fails, we must be able to go back where we
were
+ prev_sel_file = self.selected_file
+
# handling of entire paths with multiple directories/elements
if '/' in name:
- prev_sel_file = self.selected_file
pathlist = name.split('/')
# treat /DF.GSM/foo like MF/DF.GSM/foo
if pathlist[0] == '':
@@ -342,42 +366,28 @@
try:
for p in pathlist:
self.select(p, cmd_app)
- return
+ return self.selected_file_fcp
except Exception as e:
- # if any intermediate step fails, go back to where we were
self.select_file(prev_sel_file, cmd_app)
raise e
+ # we are now in the directory where the target file is located
+ # so we can now refer to the get_selectables() method to get the
+ # file object and select it using select_file()
sels = self.selected_file.get_selectables()
if is_hex(name):
name = name.lower()
- self._select_pre(cmd_app)
+ try:
+ if name in sels:
+ self.select_file(sels[name], cmd_app)
+ else:
+ self.probe_file(name, cmd_app)
+ except Exception as e:
+ self.select_file(prev_sel_file, cmd_app)
+ raise e
- if name in sels:
- f = sels[name]
- try:
- if isinstance(f, CardADF):
- (data, sw) = self.rs.card.select_adf_by_aid(f.aid,
scc=self.scc)
- self.selected_adf = f
- else:
- (data, sw) = self.scc.select_file(f.fid)
- self.selected_file = f
- except SwMatchError as swm:
- k = self.interpret_sw(swm.sw_actual)
- if not k:
- raise(swm)
- raise RuntimeError("%s: %s - %s" % (swm.sw_actual, k[0], k[1]))
- select_resp = f.decode_select_response(data)
- else:
- (select_resp, data) = self.probe_file(name, cmd_app)
-
- # store the raw + decoded FCP for later reference
- self.selected_file_fcp_hex = data
- self.selected_file_fcp = select_resp
-
- self._select_post(cmd_app)
- return select_resp
+ return self.selected_file_fcp
def status(self):
"""Request STATUS (current selected file FCP) from card."""
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34932?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I9ae213f3b078983f3e6d4c11db38fdbe504c84f2
Gerrit-Change-Number: 34932
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-MessageType: merged
