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

[bugfix] String deserialization #780

Merged
merged 9 commits into from
Jan 19, 2023

Conversation

markcutler
Copy link
Contributor

When clamping a long variable-length array I found that the jump on the string deserialization wasn't working properly. I don't fully understand what was happening, but here are the rough conditions:

  • Nested variable-length arrays.
  • The outer array was larger than the max array length. When clamped, the inner array length was decoded improperly (plotjuggler thought it was hundreds of thousands of entries when it was really only 2 or 3). This led to an out-of-memory error as we tried to decode those thousands of fictitious entries.
  • Some of the inner variable-length arrays were length-limited (e.g.: string<=1 var_name in the ros2 msg definition). I have no idea if that is related or not.
  • Decoding strings every time rather than trying to jump the appropriate number of bytes fixed the issue.

I don't know if this is the correct fix or not. The proposal seems to mirror the decoding code directly below it.

If needed, I could probably create a minimal bagfile that recreates the issue.

@facontidavide
Copy link
Owner

I don't have anything against this change. If that fix the problem, better!

Thanks a lot for contributing

@facontidavide facontidavide merged commit 0182f7b into facontidavide:main Jan 19, 2023
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.

2 participants