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=email : 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=email : 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=email : 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=email : 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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <[email protected]> Gerrit-Attention: dexter <[email protected]> Gerrit-Comment-Date: Fri, 20 Dec 2024 17:23:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
