types: fix overflow check of types/convert.go::floatStrToIntStr #11114
Conversation
|
|
Signed-off-by: H-ZeX <[email protected]>
Signed-off-by: H-ZeX <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #11114 +/- ##
================================================
+ Coverage 81.3186% 81.6933% +0.3747%
================================================
Files 423 423
Lines 90368 91699 +1331
================================================
+ Hits 73486 74912 +1426
+ Misses 11564 11483 -81
+ Partials 5318 5304 -14 |
Signed-off-by: H-ZeX <[email protected]>
|
/run-all-tests |
|
/rebuild |
|
/run-integration-tests |
| sc.AppendWarning(ErrOverflow.GenWithStackByArgs("BIGINT", oriStr)) | ||
| return validFloat[:eIdx], nil | ||
| } | ||
| intCnt += exp | ||
| if intCnt <= 0 { |
There was a problem hiding this comment.
The intCnt will never be negative according to the L448.
Prefer to:
if intCnt > 0 {
extraZeroCount := ...
} else {
...
}There was a problem hiding this comment.
intCnt>=0, exp>=0, however, if exp=I64::MAX, intCnt>0, then their sum will overflow and get negative.
There was a problem hiding this comment.
intCnt>=0, exp>=0, however, if exp=I64::MAX, intCnt>0, then their sum will overflow and get negative.
If intCnt < 0 will early return in L451.
types/convert.go
Outdated
| // (exp + incCnt) overflows MaxInt64. | ||
| intCnt += exp | ||
| // intCnt will < 0 when overflow | ||
| if intCnt < 0 || intCnt > 20 { |
There was a problem hiding this comment.
The StrToUint will use this function to extract valid int string prefix, so I think we cannot limit the length to 20.
types/convert.go
Outdated
| intStr = string(digits) + strings.Repeat("0", extraZeroCount) | ||
| } | ||
| if isOverflowInt64(intStr) { |
There was a problem hiding this comment.
I don't think we should check the overflow here because the duty of this function is to extract the valid int string prefix from a float string (the strconv.ParseInt will take charge of this, duplicate-check will cause performance issue).
Signed-off-by: H-ZeX <[email protected]>
|
/run-all-tests |
H-ZeX
changed the title
Signed-off-by: H-ZeX <[email protected]>
H-ZeX
changed the title
|
/run-all-tests |
Signed-off-by: H-ZeX [email protected]
What problem does this PR solve?
Fix overflow check of types/convert.go::floatStrToIntStr
Check List