Attention is currently required from: Hoernchen.

Patch set 2:Code-Review +1

View Change

6 comments:

• Patchset:

  • Patch Set #2:

    From what I can see, the logic is ok, a few spec references and other minor issues might be checked back. Through the unit-tests I only looked through very briefly, but they look comprehensive.

• Commit Message:

  • Patch Set #2, Line 15: That is not "redundant", as per GP CS v2.3 section 11.4.3.1 the

    I think we normally refer to the "GlobalPlatform Card
    Specification" as "GPC" (they haven't given themselves an official abbreviation - at least not that I heard of...)

• File pySim/global_platform/__init__.py:

  • Patch Set #2, Line 740: p2 = 0x02 # TLV format according to Table 11-36

    maybe reference as GPC, section 11.4.3.1, table 11-36. I mention this since at the very beginning of that section they explain the meaning of the 0x02 (bit 2 set in P2)

  • Patch Set #2, Line 751: raise SwMatchError(sw, '9000/6310')

    I would recommend to check this back. When I look at the apidoc in exceptions.py:SwMatchError, the sw_expected is defined as a string with 4 hex digits.

    I have also looked further. When using SwMatchError like this, nothing catastrophic happens. It just prints it out, but its clearly not how the inventors of SwMatchError originally thought.

    I would recommend to revisit the sw_expected parameter. Maybe we can have a variable parameter here that can be a 4 digit hex-string or a list with multiple 4 digit hex-strings. Then you would call:

    ```
    raise SwMatchError(sw, ['9000', '6310'])
    ```

    I think this would be much cleaner and future-proof.

  • Patch Set #2, Line 760: # GPC CardSpec v2.3, section 11.4.3.1, 6310 == additional matches.

    As it seems the spec ref is wrong. As far as I understand, the following is correct:

    GPC, section 11.4.2.2, table 11-34

    I wonder if we have to keep the state of bit 2 for the "Get next occurrence(s)"? Section 11.4.2.2 also mentions that bit 2 is deprecated. I would recommend to check this back.

    I haven't found the spec version 2.3, I used 2.3.1 instead, but that should be close enough.

• File pySim/transport/__init__.py:

  • Patch Set #2, Line 344: # makes the card answer 6982 and tears down the whole SCP session and following commands fail with 6985.

    I always had difficulties to understand what ETSI TS 102 221, section 7.3.1.1.4, step 4b describes, but here is what I understand now:

    We come from step 3b, and are in 3b. We send the dummy GET RESPONSE as described and we continue with step 5. Never come back, so we never send a dummy GET RESPONSE again. (all the other if statements either break or set le_gr to a length value)

    So I think that your fix is correct.

To view, visit change 43551. To unsubscribe, or for help writing mail filters, visit settings.