Fix integer conversions #14561
Fix integer conversions #14561
Conversation
twpayne
added
the
release-note/misc
|
test-me-please |
twpayne
force-pushed
the
pr/twpayne/misc-int-conversions
branch
from
deb05bb
to
1b6abf5
Compare
There was a problem hiding this comment.
LGTM!
If you update the branch for any reason, it might be worth:
- Updating the copyright of
cilium/cmd/bpf_lb_maglev_get.go. - Squashing all commits. I'm assuming you split them because they touch different areas. For this type of change, I think it's fine to use
treewide: .... That part of the commit messages doesn't impact anything in our processes anyway; the number of commits does impact the amount of work for backports, git bisect, etc.
Signed-off-by: Tom Payne <tom@isovalent.com>
twpayne
force-pushed
the
pr/twpayne/misc-int-conversions
branch
from
1b6abf5
to
aaea77a
Compare
| return block.Bytes(), 0, 0, err | ||
| } else { | ||
| block_len = int(lenUint64) |
There was a problem hiding this comment.
As a matter of style, the else block after an error return seems off, but I understand it is used here to limit the scope of the (temporary) return variables.
|
test-me-please |
|
retest-net-next |
| @@ -171,15 +171,17 @@ func extractRow(key, value string) (*metricsRow, error) { | |||
| return nil, fmt.Errorf("cannot extract reason and traffic direction from map's key \"%s\"", key) | |||
| } | |||
|
|
|||
| reasonCode, err := strconv.Atoi(reasonCodeStr) | |||
| reasonCodeUint64, err := strconv.ParseUint(reasonCodeStr, 10, 8) | |||
There was a problem hiding this comment.
Do we need to write the variable type in the name? We already know that ParseUint returns uint64s but in reality this is a uint8 so maybe would be better to rename the suffix to uint8 instead?
There was a problem hiding this comment.
strconv.ParseUint returns a uint64 whose value should fit into a uint8, but there's no actual guarantee of this provided by Go's type system. The reason for adding the Uint64 suffix to the reasonCodeUint64's variable name is to make it explicit that this is a reasonCode (which is a uint8 value) represented as a uint64 value and to make the truncating typecast later explicit.
Note that the reasonCodeUint64 variable has very limited scope, so this ugly variable name does not propagate far. Renaming reasonCodeUint64 to reasonCodeUint8 I think would cause confusion because (a) it's not a uint8 and (b) it would make the truncating typecast be uint8(reasonCodeUint8) which looks strange: why convert what seems to already be a uint8 to a uint8?
Does this reasoning make sense?
There was a problem hiding this comment.
It does yes, on one hand it's confusing to have prefixed uint64 and on the other hand it's confusing to have prefixed uint8. I'm fine either way 🙂
| @@ -251,9 +251,9 @@ func endpointToPolicyMapPath(endpointID string) (string, error) { | |||
| } | |||
|
|
|||
| var mapName string | |||
| id, err := strconv.Atoi(endpointID) | |||
| idUint64, err := strconv.ParseUint(endpointID, 10, 16) | |||
| portStr := vSplit[0] | ||
| if !iana.IsSvcName(portStr) { | ||
| var err error | ||
| port, err = strconv.Atoi(portStr) | ||
| portUint64, err := strconv.ParseUint(portStr, 10, 16) |
| _, epPortStr, err := net.SplitHostPort(epIPPort) | ||
| if err != nil { | ||
| log.WithError(err).Error("cannot extract source IP from DNS request") | ||
| } else { | ||
| if epPort, err = strconv.Atoi(epPortStr); err != nil { | ||
| if epPortUint64, err := strconv.ParseUint(epPortStr, 10, 16); err != nil { |
| } | ||
| } | ||
|
|
||
| serverIP, serverPortStr, err := net.SplitHostPort(serverAddr) | ||
| if err != nil { | ||
| log.WithError(err).Error("cannot extract destination IP from DNS request") | ||
| } else { | ||
| if serverPort, err = strconv.Atoi(serverPortStr); err != nil { | ||
| if serverPortUint64, err := strconv.ParseUint(serverPortStr, 10, 16); err != nil { |
| @@ -39,11 +39,11 @@ func C2GoArray(str string) []byte { | |||
| hexStr := strings.Split(str, ", ") | |||
| for _, hexDigit := range hexStr { | |||
| strDigit := strings.TrimPrefix(hexDigit, "0x") | |||
| digit, err := strconv.ParseInt(strDigit, 16, 9) | |||
| digitUint64, err := strconv.ParseUint(strDigit, 16, 8) | |||
| @@ -817,12 +817,12 @@ func (m *IptablesManager) GetProxyPort(name string) uint16 { | |||
| re := regexp.MustCompile(name + ".*TPROXY redirect 0.0.0.0:([1-9][0-9]*) mark") | |||
| str := re.FindString(string(res)) | |||
| portStr := re.ReplaceAllString(str, "$1") | |||
| portInt, err := strconv.Atoi(portStr) | |||
| portUInt64, err := strconv.ParseUint(portStr, 10, 16) | |||
Fix various integer conversions, as identified by CodeQL in #14514.