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 integer underflow in sendJVC(). #401

Merged
merged 1 commit into from
Jan 26, 2018
Merged

Fix integer underflow in sendJVC(). #401

merged 1 commit into from
Jan 26, 2018

Conversation

crankyoldgit
Copy link
Owner

Since sendJVC() was updated to use sendGeneric() it has effectively produced a double
minimum gap at the end of each message, instead of a single one.
This wasn't picked up by our unit-testing.
This tickled a situation where an integer underflow happened when calculating
the required space() length, which would cause a delay of approx 1h20m.
Refactored code to eliminate this problem, and a potential similar case
in sendWhynter() too.

This is a derivative of the bug/issue #381

This instance was reported in #400, which this patch should fix.

Since `sendJVC()` was updated to use `sendGeneric()` it has produced a double
minimum gap at the end of each message, instead of a single one.
This wasn't picked up by our unit-testing.
This tickled a situation where an integer underflow happened when calculating
the required `space()` length, which would cause a delay of approx 1h20m.
Refactored code to eliminate this problem, and a potential similar case
in `sendWhynter()` too.

This is a derivative of the bug/issue #381

This instance was reported in #400, which this patch should fix.
@crankyoldgit
Copy link
Owner Author

FYI @schelm87

@crankyoldgit
Copy link
Owner Author

Hey @roidayan Thanks for the prompt review!

@crankyoldgit crankyoldgit merged commit f6451bf into master Jan 26, 2018
@crankyoldgit crankyoldgit mentioned this pull request Jan 26, 2018
@crankyoldgit crankyoldgit deleted the issue-400 branch January 29, 2018 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants