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=ema... : 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=ema... : 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=ema... : 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=ema... : 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: ... ```