Skip to content
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.

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

envoyconfig: address strconv.Atoi warnings #5076

Merged
merged 1 commit into from Apr 26, 2024

Conversation

kenjenkins
Copy link
Contributor

Summary

We recently enabled CodeQL scanning. This has flagged a few instances where we parse an integer using strconv.Atoi() and then convert to a uint32. Let's resolve these warnings by using strconv.ParseUint() instead, like @calebdoxsey did in 5b06b27.

One of this instances is in a function that appears to be unused (GitHub search link), so let's go ahead and delete that function.

Related issues

User Explanation

Checklist

  • reference any related issues
  • updated docs
  • updated unit tests
  • updated UPGRADING.md
  • add appropriate tag (improvement / bug / etc)
  • ready for review

@coveralls
Copy link

coveralls commented Apr 18, 2024

Coverage Status

coverage: 57.285% (+0.03%) from 57.256%
when pulling d4d18c4 on kenjenkins/int-atoi-warning
into 498c3aa on main.

Replace Atoi() calls with ParseUint(), and update the buildAddress()
defaultPort parameter to be a uint32. (A uint16 would arguably make more
sense for a port number, but uint32 matches the Envoy proto field.)

Delete a ParseAddress() method that appears to be unused.
@kenjenkins kenjenkins marked this pull request as ready for review April 25, 2024 22:10
@kenjenkins kenjenkins requested a review from a team as a code owner April 25, 2024 22:10
@kenjenkins kenjenkins merged commit a314936 into main Apr 26, 2024
16 checks passed
@kenjenkins kenjenkins deleted the kenjenkins/int-atoi-warning branch April 26, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants