laforge has submitted this change. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/38289?usp=email ) Change subject: construct: move __bytes_required to utils and finish the implementation ...................................................................... construct: move __bytes_required to utils and finish the implementation The method __bytes_required in the GreedyInteger class computes how many byte a given integer number requires, when it is encoded. This method currently only works for unsigned integers. Let's move it to utils and add support for signed integers. Related: SYS#7094 Change-Id: I9ae4acc30dbd5fb6a6b24b10254ffb205bfde52d --- M src/osmocom/construct.py M src/osmocom/utils.py M tests/test_utils.py 3 files changed, 77 insertions(+), 21 deletions(-) Approvals: fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/src/osmocom/construct.py b/src/osmocom/construct.py index def5d85..7c0f034 100644 --- a/src/osmocom/construct.py +++ b/src/osmocom/construct.py @@ -16,7 +16,7 @@ from construct.core import evaluate from construct.lib import integertypes -from osmocom.utils import b2h, h2b, swap_nibbles +from osmocom.utils import b2h, h2b, swap_nibbles, int_bytes_required # (C) 2021-2022 by Harald Welte &lt;laforge(a)osmocom.org&gt; # @@ -587,29 +587,10 @@ except ValueError as e: raise IntegerError(str(e), path=path) - def __bytes_required(self, i, minlen=0): - if self.signed: - raise NotImplementedError("FIXME: Implement support for encoding signed integer") - - # compute how many bytes we need - nbytes = 1 - while True: - i = i >> 8 - if i == 0: - break - else: - nbytes = nbytes + 1 - - # round up to the minimum number - # of bytes we anticipate - nbytes = max(nbytes, minlen) - - return nbytes - def _build(self, obj, stream, context, path): if not isinstance(obj, integertypes): raise IntegerError(f"value {obj} is not an integer", path=path) - length = self.__bytes_required(obj, self.minlen) + length = int_bytes_required(obj, self.minlen, self.signed) try: data = obj.to_bytes(length, byteorder='big', signed=self.signed) except ValueError as e: diff --git a/src/osmocom/utils.py b/src/osmocom/utils.py index 009eb04..5947453 100644 --- a/src/osmocom/utils.py +++ b/src/osmocom/utils.py @@ -137,6 +137,44 @@ return (n + 1)//2 +def int_bytes_required(number: int, minlen:int = 0, signed:bool = False): + """compute how many bytes an integer requires when it is encoded into bytes + Args: + number : integer number + minlen : minimum length + signed : compute the number of bytes for a signed integer (two's complement) + Returns: + Integer 'nbytes', which is the number of bytes required to encode 'number' + """ + + if signed == False and number < 0: + raise ValueError("expecting a positive number") + + # Compute how many bytes we need for the absolute (positive) value of the given number + nbytes = 1 + i = abs(number) + while True: + i = i >> 8 + if i == 0: + break + else: + nbytes = nbytes + 1 + + # When we deal with signed numbers, then the two's complement applies. This means that we must check if the given + # number would still fit in the value range of the number of bytes we have calculated above. If not, one more + # byte is required. + if signed: + value_range_limit = pow(2,nbytes*8) // 2 + if number < -value_range_limit: + nbytes = nbytes + 1 + elif number >= value_range_limit: + nbytes = nbytes + 1 + + # round up to the minimum number of bytes we anticipate + nbytes = max(nbytes, minlen) + return nbytes + + def str_sanitize(s: str) -> str: """replace all non printable chars, line breaks and whitespaces, with ' ', make sure that there are no whitespaces at the end and at the beginning of the string. diff --git a/tests/test_utils.py b/tests/test_utils.py index 8e4732f..0f22e35 100755 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -54,5 +54,42 @@ self.assertEqual(str(hexstr('ABCD')), 'abcd') +class Test_int_bytes_required(unittest.TestCase): + + def test_int_bytes_required(self): + + tests = [ + # unsigned positive numbers + ( 1, 0, False ), + ( 1, 1, False ), + ( 1, 255, False ), + ( 2, 256, False ), + ( 2, 65535, False ), + ( 3, 65536, False ), + ( 2, 65535, False ), + ( 3, 16777215, False ), + ( 4, 16777216, False ), + + # signed positive numbers + ( 1, 0, True ), + ( 1, 1, True ), + ( 1, 127, True ), + ( 2, 128, True ), + ( 2, 32767, True ), + ( 3, 32768, True ), + + # signed negative numbers + ( 1, -0, True ), + ( 1, -1, True ), + ( 1, -128, True ), + ( 2, -129, True ), + ( 2, -32768, True ), + ( 3, -32769, True ), + ] + + for t in tests: + self.assertEqual(t[0], int_bytes_required(t[1], signed = t[2])) + + if __name__ == "__main__": unittest.main() -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/38289?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: I9ae4acc30dbd5fb6a6b24b10254ffb205bfde52d Gerrit-Change-Number: 38289 Gerrit-PatchSet: 2 Gerrit-Owner: dexter &lt;pmaier(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt;