fix Incorrect conversion between integer types #5626
Conversation
|
|
|
Could you add a test (and sign the CLA) ? |
| @@ -187,18 +187,20 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { // | |||
| return otts, nil | |||
| } | |||
|
|
|||
| func parseNumber(key string, input string, maximum uint8) (uint8, error) { | |||
| // Here`strconv.ParseInt` is used to parse the input string into an integer. The bit size is specified as 8, which is the bit size of `int8`. The parsed value is then checked against the maximum value, and if it's greater, an error is returned. If the parsed value is within the allowed range, it's returned as an `int8` value. | |||
There was a problem hiding this comment.
This kind of comment is too verbose imho. There's no need to describe what the method does, reading the code is the best doc.
|
/easycla |
|
Hi @dmathieu, we've signed the CLA and are configured in EasyCLA to accept anyone who is a member of our public-facing org, but for some reason it's not picking up @Nanmozhi22. I've filed a ticket with support asking for help. |
| if err != nil { | ||
| return maximum + 1, parseError(key, err) | ||
| } | ||
| if value > uint64(maximum) { | ||
| if value > int64(maximum) { |
There was a problem hiding this comment.
Comparing int8(value) with int64(maximum) can I know why?
MrAlias
added
the
blocked: CLA
| func parseNumber(key string, input string, maximum uint8) (uint8, error) { | ||
| // Here`strconv.ParseInt` is used to parse the input string into an integer. The bit size is specified as 8, which is the bit size of `int8`. The parsed value is then checked against the maximum value, and if it's greater, an error is returned. If the parsed value is within the allowed range, it's returned as an `int8` value. | ||
|
|
||
| func parseNumber(key string, input string, maximum int8) (int8, error) { |
There was a problem hiding this comment.
This change doesn't make sense. The output of this is going to be used as a uint8 anyway, so we are now throwing away half the usable space. This change will break builds that haven't been run.
There was a problem hiding this comment.
Thanks for the response! Do you have any suggestions on how to make it , so that I can try to make those changes
| if value > uint64(maximum) { | ||
| if value > int64(maximum) { | ||
| return maximum + 1, parseError(key, strconv.ErrRange) | ||
| } | ||
| return uint8(value), nil | ||
| return int8(value), nil |
There was a problem hiding this comment.
This comparison will prevent the uint8 conversion from ever overflowing. Furthermore, this is always called with maximum = 62, so we aren't near overflow anyways.
No description provided.