- gerrit-log - lists.osmocom.org

[S] Change in osmo-hnbgw[master]: hnbgw_rx_hnb_register_req(): guard against asn.1 parsing errors
by fixeria 20 Jan '25

20 Jan '25

Attention is currently required from: neels. fixeria has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/39362?usp=email ) Change subject: hnbgw_rx_hnb_register_req(): guard against asn.1 parsing errors ...................................................................... Patch Set 2: (1 comment) File src/osmo-hnbgw/hnbgw_hnbap.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/39362/comment/afb5d012_843b807f?u… : PS2, Line 494: make sure that actual values ended up in the asn1 octet strings Are these fields marked as `MANDATORY` in the respective ASN.1 definition? If so, then I would expect `hnbap_decode_hnbregisterrequesties()` to fail (return negative) if at least one `MANDATORY` field/IE is somehow missing. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/39362?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I555f11cadc23ea231821bd48f7cd62953b022e9e Gerrit-Change-Number: 39362 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 13:00:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in osmo-trx[master]: comma_delimited_to_vector() optimization CID#465430
by fixeria 20 Jan '25

20 Jan '25

Attention is currently required from: Hoernchen, neels. fixeria has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/osmo-trx/+/39359?usp=email ) Change subject: comma_delimited_to_vector() optimization CID#465430 ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-trx/+/39359/comment/15761bdc_2aa9559d?usp… : PS1, Line 9: (I The patch looks good. Moving is "cheaper" than copying. See https://lemire.me/blog/2023/01/30/move-or-copy-your-strings-possible-perfor…, for instance. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/39359?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I30e57c6a9df39552bc4f8c18d27bc421f5c60f49 Gerrit-Change-Number: 39359 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 12:57:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-ci[master]: jobs: manually trigger "git gc" for linux.git
by fixeria 20 Jan '25

20 Jan '25

Attention is currently required from: osmith. fixeria has posted comments on this change by osmith. ( https://gerrit.osmocom.org/c/osmo-ci/+/39373?usp=email ) Change subject: jobs: manually trigger "git gc" for linux.git ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/39373?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: Ic81ae5a1edb0dbe119e06cc3b49ea69ef81b2a5f Gerrit-Change-Number: 39373 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 12:50:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-ci[master]: jobs: manually trigger "git gc" for linux.git
by osmith 20 Jan '25

20 Jan '25

osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ci/+/39373?usp=email ) Change subject: jobs: manually trigger "git gc" for linux.git ...................................................................... jobs: manually trigger "git gc" for linux.git We already use shallow clones for linux.git, and git runs a garbage collector automatically to ensure no longer needed artifacts are removed. But this did not work anymore on build4 for build-kernel-net-next, and it increased up to 8.4 GiB: 8,4 GiB /build-kernel-net-next After running "git gc" again: 3,3 GiB /build-kernel-net-next The reason was that "git gc" did not exit successfully once and wrote a log file, then refused to run again. Run "git gc --auto" explicitly in foreground to let abort in such a case, so it does not go unnoticed in the future. Change-Id: Ic81ae5a1edb0dbe119e06cc3b49ea69ef81b2a5f --- M jobs/build-kernels-testenv.yml M jobs/gerrit-verifications-dahdi.yml M jobs/master-builds-dahdi.yml 3 files changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ci refs/changes/73/39373/1 diff --git a/jobs/build-kernels-testenv.yml b/jobs/build-kernels-testenv.yml index d916093..47a9dd9 100644 --- a/jobs/build-kernels-testenv.yml +++ b/jobs/build-kernels-testenv.yml @@ -43,6 +43,9 @@ default: 'master' builders: - shell: |- + git config gc.autoDetach false + git gc --auto + IMAGE="osmocom-build/debian-bookworm-osmo-ttcn3-testenv" podman pull "registry.osmocom.org/$IMAGE" diff --git a/jobs/gerrit-verifications-dahdi.yml b/jobs/gerrit-verifications-dahdi.yml index 2cf9e95..de11b25 100644 --- a/jobs/gerrit-verifications-dahdi.yml +++ b/jobs/gerrit-verifications-dahdi.yml @@ -80,6 +80,9 @@ builders: - shell: | + git -C linux config gc.autoDetach false + git -C linux gc --auto + rm -rf dahdi-linux git init dahdi-linux cd dahdi-linux diff --git a/jobs/master-builds-dahdi.yml b/jobs/master-builds-dahdi.yml index 8f6d070..f72076f 100644 --- a/jobs/master-builds-dahdi.yml +++ b/jobs/master-builds-dahdi.yml @@ -56,6 +56,9 @@ builders: - shell: | + git -C linux config gc.autoDetach false + git -C linux gc --auto + rm -rf dahdi-linux git clone \ --depth=1 \ -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/39373?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: Ic81ae5a1edb0dbe119e06cc3b49ea69ef81b2a5f Gerrit-Change-Number: 39373 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de>

1 0

[S] Change in pysim[master]: global_platform: fix usage of the Key Version Number (kvn)
by laforge 20 Jan '25

20 Jan '25

laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/39225?usp=email ) Change subject: global_platform: fix usage of the Key Version Number (kvn) ...................................................................... global_platform: fix usage of the Key Version Number (kvn) The kvn parameter is used to select a keyset when establishin a secure channel. At the moment this is a mandatory parameter and it must be within a certain range. However GPC_SPE_034 explicitly defines a reserved kvn value 0, that always refers to the first available key. That effectively makes it an optional parameter and the commandline interface should have the --key-ver parameter as an optional parameter. The ranges also have to be extended to allow 0 as kvn value. We also have to put a range to support the sysmoUSIM-SJS1, which uses kvn value 1, which is a non standard value. Related: OS#6679 Change-Id: I42be2438c7f199b238f2ec7a9434cec5393210a7 --- M pySim/global_platform/__init__.py M pySim/global_platform/scp.py 2 files changed, 40 insertions(+), 5 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/pySim/global_platform/__init__.py b/pySim/global_platform/__init__.py index c0607ae..f2bec81 100644 --- a/pySim/global_platform/__init__.py +++ b/pySim/global_platform/__init__.py @@ -795,7 +795,7 @@ return self._cmd.lchan.scc.send_apdu_checksw(cmd_hex) est_scp02_parser = argparse.ArgumentParser() - est_scp02_parser.add_argument('--key-ver', type=auto_uint8, required=True, help='Key Version Number (KVN)') + est_scp02_parser.add_argument('--key-ver', type=auto_uint8, default=0, help='Key Version Number (KVN)') est_scp02_parser.add_argument('--host-challenge', type=is_hexstr, help='Hard-code the host challenge; default: random') est_scp02_parser.add_argument('--security-level', type=auto_uint8, default=0x01, @@ -900,7 +900,9 @@ class GpCardKeyset: """A single set of GlobalPlatform card keys and the associated KVN.""" def __init__(self, kvn: int, enc: bytes, mac: bytes, dek: bytes): - assert 0 < kvn < 256 + # The Key Version Number is an 8 bit integer number, where 0 refers to the first available key, + # see also: GPC_SPE_034, section E.5.1.3 + assert 0 <= kvn < 256 assert len(enc) == len(mac) == len(dek) self.kvn = kvn self.enc = enc diff --git a/pySim/global_platform/scp.py b/pySim/global_platform/scp.py index 3fe7601..72aed24 100644 --- a/pySim/global_platform/scp.py +++ b/pySim/global_platform/scp.py @@ -113,7 +113,39 @@ class SCP(SecureChannel, abc.ABC): """Abstract base class containing some common interface + functionality for SCP protocols.""" def __init__(self, card_keys: 'GpCardKeyset', lchan_nr: int = 0): - if hasattr(self, 'kvn_range'): + + # Spec references that explain KVN ranges: + # TS 102 225 Annex A.1 states KVN 0x01..0x0F shall be used for SCP80 + # GPC_GUI_003 states + # * For the Issuer Security Domain, this is initially Key Version Number 'FF' which has been deliberately + # chosen to be outside of the allowable range ('01' to '7F') for a Key Version Number. + # * It is logical that the initial keys in the Issuer Security Domain be replaced by an initial issuer Key + # Version Number in the range '01' to '6F'. + # * Key Version Numbers '70' to '72' and '74' to '7F' are reserved for future use. + # * On an implementation supporting Supplementary Security Domains, the RSA public key with a Key Version + # Number '73' and a Key Identifier of '01' has the following functionality in a Supplementary Security + # Domain with the DAP Verification privilege [...] + # GPC_GUI_010 V1.0.1 Section 6 states + # * Key Version number range ('20' to '2F') is reserved for SCP02 + # * Key Version 'FF' is reserved for use by an Issuer Security Domain supporting SCP02, and cannot be used + # for SCP80. This initial key set shall be replaced by a key set with a Key Version Number in the + # ('20' to '2F') range. + # * Key Version number range ('01' to '0F') is reserved for SCP80 + # * Key Version number '70' with Key Identifier '01' is reserved for the Token Key, which is either a RSA + # public key or a DES key + # * Key Version number '71' with Key Identifier '01' is reserved for the Receipt Key, which is a DES key + # * Key Version Number '11' is reserved for DAP as specified in ETSI TS 102 226 [2] + # * Key Version Number '73' with Key Identifier '01' is reserved for the DAP verification key as specified + # in sections 3.3.3 and 4 of [4], which is either an RSA public key or DES key + # * Key Version Number '74' is reserved for the CASD Keys (cf. section 9.2) + # * Key Version Number '75' with Key Identifier '01' is reserved for the key used to decipher the Ciphered + # Load File Data Block described in section 4.8 of [5]. + + if card_keys.kvn == 0: + # Key Version Number 0x00 refers to the first available key, so we won't carry out + # a range check in this case. See also: GPC_SPE_034, section E.5.1.3 + pass + elif hasattr(self, 'kvn_range'): if not card_keys.kvn in range(self.kvn_range[0], self.kvn_range[1]+1): raise ValueError('%s cannot be used with KVN outside range 0x%02x..0x%02x' % (self.__class__.__name__, self.kvn_range[0], self.kvn_range[1])) @@ -224,8 +256,9 @@ constr_iur = Struct('key_div_data'/Bytes(10), 'key_ver'/Int8ub, Const(b'\x02'), 'seq_counter'/Int16ub, 'card_challenge'/Bytes(6), 'card_cryptogram'/Bytes(8)) - # The 0x70 is a non-spec special-case of sysmoISIM-SJA2/SJA5 and possibly more sysmocom products - kvn_ranges = [[0x20, 0x2f], [0x70, 0x70]] + # Key Version Number 0x70 is a non-spec special-case of sysmoISIM-SJA2/SJA5 and possibly more sysmocom products + # Key Version Number 0x01 is a non-spec special-case of sysmoUSIM-SJS1 + kvn_ranges = [[0x01, 0x01], [0x20, 0x2f], [0x70, 0x70]] def __init__(self, *args, **kwargs): self.overhead = 8 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39225?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I42be2438c7f199b238f2ec7a9434cec5393210a7 Gerrit-Change-Number: 39225 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org>

1 0

[S] Change in pysim[master]: global_platform: fix usage of the Key Version Number (kvn)
by laforge 20 Jan '25

20 Jan '25

Attention is currently required from: dexter. laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/39225?usp=email ) Change subject: global_platform: fix usage of the Key Version Number (kvn) ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39225?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I42be2438c7f199b238f2ec7a9434cec5393210a7 Gerrit-Change-Number: 39225 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 12:42:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-ttcn3-hacks[master]: gbproxy: Don't log the payload size for every DL/UL-UNITDATA
by laforge 20 Jan '25

20 Jan '25

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39372?usp=email ) Change subject: gbproxy: Don't log the payload size for every DL/UL-UNITDATA ...................................................................... gbproxy: Don't log the payload size for every DL/UL-UNITDATA This just wastes memory. We can check the pcaps if a test does fail. Change-Id: I862d67a48f1d694262cf39a1e939033bde0c6eab Related: OS#6704 --- M gbproxy/GBProxy_Tests.ttcn 1 file changed, 0 insertions(+), 3 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/gbproxy/GBProxy_Tests.ttcn b/gbproxy/GBProxy_Tests.ttcn index 7a6572a..7f70f96 100644 --- a/gbproxy/GBProxy_Tests.ttcn +++ b/gbproxy/GBProxy_Tests.ttcn @@ -1231,7 +1231,6 @@ /* we cannot use pdu_tx as there are some subtle differences in the length field :/ */ var template (present) PDU_BSSGP pdu_rx := tr_BSSGP_UL_UD(g_pars.tlli, bvcc.cell_id, payload); - log("UL-UNITDATA(payload_size=", i); f_pcu2sgsn(pdu_tx, pdu_rx); } setverdict(pass); @@ -1261,7 +1260,6 @@ /* we cannot use pdu_tx as there are some subtle differences in the length field :/ */ var template (present) PDU_BSSGP pdu_rx := tr_BSSGP_UL_UD(g_pars.tlli, bvcc.cell_id, payload); - log("UL-UNITDATA(payload_size=", i); f_pcu2sgsn(pdu_tx, pdu_rx); } setverdict(pass); @@ -1319,7 +1317,6 @@ var template (present) PDU_BSSGP pdu_rx := tr_BSSGP_DL_UD(g_pars.tlli, payload, tr_BSSGP_IMSI(g_pars.imsi)); - log("DL-UNITDATA(payload_size=", i, ")"); f_sgsn2pcu(pdu_tx, pdu_rx); } setverdict(pass); -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39372?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I862d67a48f1d694262cf39a1e939033bde0c6eab Gerrit-Change-Number: 39372 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-ttcn3-hacks[master]: gbproxy: Don't log the payload size for every DL/UL-UNITDATA
by laforge 20 Jan '25

20 Jan '25

Attention is currently required from: daniel. laforge has posted comments on this change by daniel. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39372?usp=email ) Change subject: gbproxy: Don't log the payload size for every DL/UL-UNITDATA ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39372?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I862d67a48f1d694262cf39a1e939033bde0c6eab Gerrit-Change-Number: 39372 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 12:39:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in pysim[master]: global_platform: add new command "install_cap"
by fixeria 20 Jan '25

20 Jan '25

Attention is currently required from: dexter, laforge. fixeria has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/39199?usp=email ) Change subject: global_platform: add new command "install_cap" ...................................................................... Patch Set 7: (6 comments) File docs/cap-tutorial.rst: https://gerrit.osmocom.org/c/pysim/+/39199/comment/e70f3a56_000afc68?usp=em… : PS7, Line 5: support "do support ... support" https://gerrit.osmocom.org/c/pysim/+/39199/comment/3b844dd9_e0495a8a?usp=em… : PS7, Line 54: "maximum_length_of_data_field_in_command_message": 255 shouldn't there be an additional indention level? File pySim/global_platform/__init__.py: https://gerrit.osmocom.org/c/pysim/+/39199/comment/151e93f6_b121b29e?usp=em… : PS7, Line 868: install Parameters (GPC_SPE_034, section 11.5.2.3.7, table 11-49 looks like copy paste. Maybe clarify that it's one of the installation params setting the volatile memory quota? https://gerrit.osmocom.org/c/pysim/+/39199/comment/e771db3a_fe185277?usp=em… : PS7, Line 865: install_cap_parser.add_argument('--install-parameters', type=is_hexstr, default='', : help='install Parameters (GPC_SPE_034, section 11.5.2.3.7, table 11-49)') : install_cap_parser.add_argument('--install-parameters-volatile-memory-quota', type=int, default=None, : help='install Parameters (GPC_SPE_034, section 11.5.2.3.7, table 11-49)') : install_cap_parser.add_argument('--install-parameters-non-volatile-memory-quota', type=int, default=None, : help='Install Parameters (GPC_SPE_034, section 11.5.2.3.7, table 11-49)') : install_cap_parser.add_argument('--install-parameters-stk', type=is_hexstr, default=None, : Like in previous patch, define them as mutually exclusive params? https://docs.python.org/3/library/argparse.html#mutual-exclusion. File pySim/global_platform/install_param.py: https://gerrit.osmocom.org/c/pysim/+/39199/comment/525ddd5a_7316dc97?usp=em… : PS7, Line 20: from construct import Optional as COptional : fr none of these used? File tests/unittests/test_globalplatform.py: https://gerrit.osmocom.org/c/pysim/+/39199/comment/7cf3087c_1252dfb5?usp=em… : PS7, Line 24: * you only need `gen_install_parameters()`, so import it alone? Otherwise you're importing everything, including imports of the given module. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39199?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I6cbd37f0fad5579b20e83c27349bd5acc129e6d0 Gerrit-Change-Number: 39199 Gerrit-PatchSet: 7 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 12:37:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in pysim[master]: global_platform: LOAD and INSTALL [for load] support
by fixeria 20 Jan '25

20 Jan '25

Attention is currently required from: dexter, laforge. fixeria has posted comments on this change by laforge. ( https://gerrit.osmocom.org/c/pysim/+/37454?usp=email ) Change subject: global_platform: LOAD and INSTALL [for load] support ...................................................................... Patch Set 7: (6 comments) File pySim/global_platform/__init__.py: https://gerrit.osmocom.org/c/pysim/+/37454/comment/9e121e2d_cb2e3367?usp=em… : PS7, Line 20: import io unused import? (hm, why doesn't the linter complain?) https://gerrit.osmocom.org/c/pysim/+/37454/comment/622c3a16_f8831d53?usp=em… : PS7, Line 772: Prefixed(Int8ub, GreedyBytes This is basically LV structure. Given that it's used several times and can be useful in other places, maybe define it somewhere? Wait, actually it's already defined in pyosmocom: ``` # here we collect some shared / common definitions of data types LV = Prefixed(Int8ub, HexAdapter(GreedyBytes)) ``` It's a bit different, but I assume it works the same way? https://gerrit.osmocom.org/c/pysim/+/37454/comment/9020f333_a7771748?usp=em… : PS7, Line 824: ('--from-hex why not using `type=is_hexstr` here? https://gerrit.osmocom.org/c/pysim/+/37454/comment/391df02d_45717cef?usp=em… : PS7, Line 824: load_parser.add_argument('--from-hex', help='load from hex string') : load_parser.add_argument('--from-file', help='load from binary file') : If these params are mutually-exclusive (only one of these must be supplied), please create a group using `add_mutually_exclusive_group()` and make that group `mandatory=True`. https://gerrit.osmocom.org/c/pysim/+/37454/comment/e066d0cc_23d5c223?usp=em… : PS7, Line 833: formatting https://gerrit.osmocom.org/c/pysim/+/37454/comment/5ededfac_4e9b9779?usp=em… : PS7, Line 835: with open(opts.from_file, 'rb') as f: BTW, argparse does have: https://docs.python.org/3/library/argparse.html#filetype-objects -- To view, visit https://gerrit.osmocom.org/c/pysim/+/37454?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I924aaeecbb3a72bdb65eefbff6135e4e9570579e Gerrit-Change-Number: 37454 Gerrit-PatchSet: 7 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 12:13:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

2026

2025

2024

2023

2022

gerrit-log