dexter has uploaded this change for review. ( 
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#6210
Change-Id: I9ae213f3b078983f3e6d4c11db38fdbe504c84f2
---
M pySim/runtime.py
1 file changed, 89 insertions(+), 41 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/32/34932/1

diff --git a/pySim/runtime.py b/pySim/runtime.py
index 8660724..feaa355 100644
--- a/pySim/runtime.py
+++ b/pySim/runtime.py
@@ -64,7 +64,7 @@
                     self.mf.add_file(f)

         # go back to MF before the next steps (addon probing might have 
changed DF)
-        self.card._scc.select_file('3F00')
+        self.lchan[0].select('3F00')

         # add application ADFs + MF-files from profile
         apps = self._match_applications()
@@ -110,6 +110,12 @@
         # probe for those applications
         for f in sorted(set(apps_profile) - set(apps_taken), key=str):
             try:
+                # we can not use the lchan provided methods select, or 
select_file
+                # since those method work on an already finished file model. At
+                # this point we are still in the initialization process, so it 
is
+                # no problem when we access the card directly without caring 
about
+                # updating other states. For normal selects at runtime, the 
caller
+                # must use the lchan provided methods select or select_file!
                 data, sw = self.card.select_adf_by_aid(f.aid)
                 if sw == "9000":
                     print(" %s: %s" % (f.name, f.aid))
@@ -163,11 +169,14 @@
     def __init__(self, lchan_nr: int, rs: RuntimeState):
         self.lchan_nr = lchan_nr
         self.rs = rs
+        self.scc = self.rs.card._scc.fork_lchan(lchan_nr)
+
+       # File reference data
         self.selected_file = self.rs.mf
         self.selected_adf = None
         self.selected_file_fcp = None
         self.selected_file_fcp_hex = None
-        self.scc = self.rs.card._scc.fork_lchan(lchan_nr)
+

     def add_lchan(self, lchan_nr: int) -> 'RuntimeLchan':
         """Add a new logical channel from the current logical channel. Just 
affects
@@ -246,9 +255,17 @@
             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 and we may proceed 
with
+            # creating a new file at run time. In case the file does not 
exist, we
+            # just abort. The state on the card (selected file/application) 
wont't
+            # be changed, 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)
@@ -267,8 +284,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
@@ -276,7 +293,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: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:
@@ -293,22 +320,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_port. 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, ...).
@@ -317,9 +353,11 @@
             name : Name of file to select
             cmd_app : Command Application State (for unregistering old file 
commands)
         """
+        # if any intermediate step failes, 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] == '':
@@ -327,41 +365,29 @@
             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:
+                f = sels[name]
+                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)
-                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: 1
Gerrit-Owner: dexter <pma...@sysmocom.de>
Gerrit-MessageType: newchange

Reply via email to