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 SignalPdu unmarshal size issue #66

Merged
merged 1 commit into from
Aug 9, 2019
Merged

Conversation

Serubin
Copy link
Contributor

@Serubin Serubin commented Aug 1, 2019

SignalPdu unmarshal currently relies on using the data bucket size to calculate the unmarshal byte amount. This size is not consistent as the data bucket size may not represent any padded bytes included. Thus using the header size is safer and more consistent.

Don't rely on data bucket to calculate size - it's not consistent
@leif81
Copy link
Member

leif81 commented Aug 5, 2019

Thanks @Serubin

I'm a little confused now why we have getLength and getPduLength. Certainly room for confusion if nothing else.

@Serubin
Copy link
Contributor Author

Serubin commented Aug 5, 2019

I absolutely agree. getLength calculates based on the size of the data coming in, whereas getPduLength retrieves the size from the PDU header. Getting the size from header is almost always more reliable.

There may be some use for getLength but it's not in the unmarshal process. Took me a while to determine why my bundled PDUs were consistently starting one byte off from their actual beginning.

@leif81
Copy link
Member

leif81 commented Aug 9, 2019

Sorry I was away camping for a few days.

Lets merge this as is and we can think about refactoring getLength/getPduLength methods outside of this PR.

@leif81 leif81 merged commit bfd132c into open-dis:master Aug 9, 2019
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