CL33-09: Make seq num range resilient to mixed up beginning and end.#404
CL33-09: Make seq num range resilient to mixed up beginning and end.#404winder wants to merge 2 commits into
Conversation
winder
requested review from
0xAustinWang,
asoliman92,
b-gopalswami,
makramkd,
mateusz-sekara and
rstout
|
| } | ||
|
|
||
| func NewSeqNumRange(start, end SeqNum) SeqNumRange { | ||
| if end < start { |
There was a problem hiding this comment.
should we even allow this to begin with?
There was a problem hiding this comment.
I went back and forth about this question. This could generate an error but would require updating >150 spots where we use it.
| } | ||
|
|
||
| func (s SeqNumRange) Length() int { | ||
| if s.End() < s.Start() { |
There was a problem hiding this comment.
This check is probably not needed considering you've already covered that on the constructor level
There was a problem hiding this comment.
Also, it's misleading to shuffle with the state in the read method. If you need a defensive check then probably calling absolute value on the computed length should work, no?
| } | ||
|
|
||
| func NewSeqNumRange(start, end SeqNum) SeqNumRange { | ||
| if end < start { |
There was a problem hiding this comment.
Hmm, I'm not sure about that. Shouldn't we error when someone mixes start with the end? We could hide some underlying bug when "healing" that state during runtime
|
This still relevant @winder ? |
…o-tel-collector-deployment-without-operator feat(beholder): migrate helm chart from operator to helm chart
4 participants
Avoid overflow when End < Start by swapping values to be in the correct order.