Skip to content

Keep poll and precision intervals inside a duration #57

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.

Sign up for GitHub

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

Open
SashaMIT wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Keep poll and precision intervals inside a duration #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions ntp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,12 +919,21 @@ func toTimestamp(t time.Time) timestamp {
}

// toInterval converts an NTP poll interval exponent into a time.Duration.
// The exponent is an int8 from the packet. Negating -128 overflows, and the
// following shift panics. A shift of 34 or more does not fit in a duration
// and wraps negative. Those exponents stay at the nearest duration that fits.
func toInterval(t int8) time.Duration {
const maxShift = 33 // time.Second<<34 overflows int64
shift := int(t)
switch {
case t > 0:
return time.Duration(int64(time.Second) << int(t))
case t < 0:
return time.Duration(int64(time.Second) >> int(-t))
case shift > maxShift:
return time.Second << maxShift
case shift > 0:
return time.Second << shift
case shift < -29:

@beevik beevik Oct 10, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, let's use a minShift constant.

@beevik beevik Oct 10, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the more I think about it, the more I think this function shouldn't be allowed to return zero. From the client's perspective, they're interested in the poll interval or the precision. In neither case is an interval of zero particularly helpful.

Perhaps the function should be more like this:

func toInterval(t int8) time.Duration {
	const minShift = -29
	const maxShift = 33
	shift := min(maxShift, max(minShift, int(t)))
	switch {
	case shift > 0:
		return time.Duration(int64(time.Second) << shift)
	case shift < 0:
		return time.Duration(int64(time.Second) >> -shift)
	default:
		return time.Second
	}
}

return 0
case shift < 0:
return time.Second >> -shift
default:
return time.Second
}
Expand Down
11 changes: 11 additions & 0 deletions ntp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,3 +297,14 @@ func TestOfflineKissCode(t *testing.T) {
assert.Equal(t, kissCode(c.id), c.str)
}
}

func TestToIntervalBounds(t *testing.T) {
assert.Equal(t, time.Second, toInterval(0))
assert.Equal(t, 16*time.Second, toInterval(4))
assert.Equal(t, 500*time.Millisecond, toInterval(-1))
assert.Equal(t, time.Nanosecond, toInterval(-29))
assert.Equal(t, time.Duration(0), toInterval(-30))
assert.Equal(t, time.Duration(0), toInterval(-128))
assert.Equal(t, time.Second<<33, toInterval(34))
assert.Equal(t, time.Second<<33, toInterval(127))
}