Attention is currently required from: dexter.

laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38195?usp=email )

Change subject: filesystem: pass total_len to construct of when encoding file contents ......................................................................

Patch Set 2:

(1 comment)

File pySim/filesystem.py:

https://gerrit.osmocom.org/c/pysim/+/38195/comment/fb0a4300_36e58f66?usp=ema... : PS2, Line 1338: def _decode_bin(self, raw_bin_data: bytearray): : chunks = [raw_bin_data[i:i+self.rec_len] : for i in range(0, len(raw_bin_data), self.rec_len)] : return [self.decode_record_bin(x) for x in chunks] : : def _encode_bin(self, abstract_data) -> bytes: : chunks = [self.encode_record_bin(x) for x in abstract_data] : # FIXME: pad to file size : return b''.join(chunks) you are not passing the total_len as argument to the self.{encode,decode}_record_bin here. This means the actual file size will not be used, and we will always call self.__get_rec_len(None)

In order to have the total_len available, the calls to _{encode,decode}_bin must be extended with an additional argument, just like the build_construct gets passed the context. (note: there are likely other derived classes that have their own _{en,de}code_bin methods, as they don't use _construct. So you need to change all 'def _{en,de}code_bin' and 'def _{en,de}code_hex' methods of all derived classes to accept the extra argument.

This doesn't cause an error in the test suite, as we don't have tests where the file size is larger than the data we want to write to it. All our tests have fixed-size records and decode/encode to the exact same size, so you don't ever use this.*total_len in the _construct.

So IMHO it's just a lack of test coverage that hides the bug here.