Attention is currently required from: pespin.
laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39170?usp=email )
Change subject: pcap-client: Decouple dev capture handling from client
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39170?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-Change-Id: I02f33f11cbcb7ed5ff6475df9442741618178379
Gerrit-Change-Number: 39170
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 20 Dec 2024 17:35:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: dexter.
laforge 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 1: Code-Review+1
--
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: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 20 Dec 2024 17:24:03 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
laforge 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
......................................................................
Set Ready For Review
--
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: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Comment-Date: Fri, 20 Dec 2024 17:23:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Attention is currently required from: dexter.
laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/39198?usp=email )
Change subject: javacard: add parser for JAVA-card CAP file format
......................................................................
Patch Set 1:
(4 comments)
File pySim/javacard.py:
https://gerrit.osmocom.org/c/pysim/+/39198/comment/e3f8a363_d43eb964?usp=em… :
PS1, Line 45:
can we somehow check easily if a file is extended and in that case print a meaningful error message?
https://gerrit.osmocom.org/c/pysim/+/39198/comment/2b0437ce_47f40f21?usp=em… :
PS1, Line 47: __component_header = None
: __component_directory = None
: __component_applet = None #optional
: __component_import = None
: __component_constantPool = None
: __component_class = None
: __component_method = None
: __component_staticField = None
: __component_referenceLocation = None
: __component_export = None #optional
: __component_descriptor = None
: __component_debug = None #optional, since CAP format 2.2
doesn't it make sense to use a dict here, indexed by the component type? Just a thought.
https://gerrit.osmocom.org/c/pysim/+/39198/comment/a3afdc52_db227642?usp=em… :
PS1, Line 86: if filename.lower().endswith('header.cap'):
: self.__component_header = cap.read(filename)
: elif filename.lower().endswith('directory.cap'):
: self.__component_directory = cap.read(filename)
: elif filename.lower().endswith('applet.cap'):
: self.__component_applet = cap.read(filename)
: elif filename.lower().endswith('import.cap'):
: self.__component_import = cap.read(filename)
: elif filename.lower().endswith('constantpool.cap'):
: self.__component_constantPool = cap.read(filename)
: elif filename.lower().endswith('class.cap'):
: self.__component_class = cap.read(filename)
: elif filename.lower().endswith('method.cap'):
: self.__component_method = cap.read(filename)
: elif filename.lower().endswith('staticfield.cap'):
: self.__component_staticField = cap.read(filename)
: elif filename.lower().endswith('reflocation.cap'):
: self.__component_referenceLocation = cap.read(filename)
: elif filename.lower().endswith('export.cap'):
: self.__component_export = cap.read(filename)
: elif filename.lower().endswith('descriptor.cap'):
: self.__component_descriptor = cap.read(filename)
: elif filename.lower().endswith('debug.cap'):
: self.__component_debug = cap.read(filename)
IMHO, this looks like a lot of copy+paste / typing work in something that can be implemented more elegantly and programmatically:
```
if not filename.lower().endswith('.cap')
continue
setattr(self, '__component_' + filename.removesuffix(.cap), cap.read(filename))
```
https://gerrit.osmocom.org/c/pysim/+/39198/comment/af30a855_e085077b?usp=em… :
PS1, Line 113: if self.__component_header is None:
: raise ValueError("invalid cap file, COMPONENT_Header missing!")
: if self.__component_directory is None:
: raise ValueError("invalid cap file, COMPONENT_Directory missing!")
: if self.__component_import is None:
: raise ValueError("invalid cap file, COMPONENT_Import missing!")
: if self.__component_constantPool is None:
: raise ValueError("invalid cap file, COMPONENT_ConstantPool missing!")
: if self.__component_class is None:
: raise ValueError("invalid cap file, COMPONENT_Class missing!")
: if self.__component_method is None:
: raise ValueError("invalid cap file, COMPONENT_Method missing!")
: if self.__component_staticField is None:
: raise ValueError("invalid cap file, COMPONENT_StaticField missing!")
: if self.__component_referenceLocation is None:
: raise ValueError("invalid cap file, COMPONENT_ReferenceLocation missing!")
: if self.__component_descriptor is None:
: raise ValueError("invalid cap file, COMPONENT_Descriptor missing!")
likewise here, that's a lot of code to write. Why not simply have a
```
required_components = ['header', 'directory', ...]
for comp in required_components:
...
```
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39198?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: I581483ccb9d8a254fcecc995fec3c811c5cf38eb
Gerrit-Change-Number: 39198
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 20 Dec 2024 17:23:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No