Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix kaitai python version for messages ending in a variable-length string #1468

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dgburr
Copy link
Contributor

@dgburr dgburr commented Jan 17, 2025

Description

The kaitai python bindings contain an internal helper class called BufferKaitaiStream, which is intended to be a faster work-alike for the KaitaiStream class provided by the kaitai python runtime. This is implemented using an inner class called KaitaiStream.IOBytes, which emulates the io.BytesIO interface expected by KaitaiStream. Unfortunately the previous implementation of BufferKaitaiStruct.BytesIO.read() contained a bug which meant that it did not correctly handle messages (such as MSG_DGNSS_STATUS and MSG_PROFILING_THREAD_INFO) which end in a variable-length string. In this case, KaitaiStream.read_bytes_full() would call BufferKaitaiStruct.BytesIO.read() without any arguments, with the expectation that it would return all of the data until the end of the message.

The majority of this PR is concerned with creating a test case for using the kaitai python bindings which completely avoids the use of io.BytesIO abstraction. According to the benchmark this approach is approximately 15% faster than both of the other hybrid variants.

@swift-nav/algint-team

API compatibility

Does this change introduce a API compatibility risk?

No, it is only a bug fix.

API compatibility plan

If the above is "Yes", please detail the compatibility (or migration) plan:

@dgburr dgburr requested a review from a team as a code owner January 17, 2025 23:56
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant