fix codeql alerts #9647
fix codeql alerts #9647
Conversation
mx-psi
added
changelog/no-changelog
[deprecated] team/agent-core
|
@stephengroat-dd you need to sign the CLA for this to be mergeable |
|
@mx-psi Thanks for letting us know. All committers have signed the CLA now :) |
|
FYI: I pushed a copy of this branch to |
| // Before Go 1.17, we can use the following trick to define MaxInt | ||
| const MaxInt = ^uint(0) >> 1 | ||
| if err != nil || titleLength < 0 || titleLength > int64(MaxInt) { |
There was a problem hiding this comment.
As titleLength is an int64, there is no possible value that can be greater than MaxInt.
So, titleLength > int64(MaxInt) is an antilogy, isn’t it ?
There was a problem hiding this comment.
On 64-bit machines (including that used for the playground), you are correct. In a 32-bit context, int is the same as int32, so there are lots of int64 values that are greater than MaxInt.
This is one of those holdovers in Go from the C/C++ world that has caused lots of bugs in C/C++ and continues to do so in Go.
There was a problem hiding this comment.
parseInt64 is only used in this function. So I think we can change parseInt64() int64 to parseUint() uint (adjusting implementation accordingly) and avoid the need for range checks here. WDYT?
vickenty
added
the
[deprecated] qa/skip-qa - use other qa/ labels
|
I pinged some slack channels for the remaining two teams. It'd be great to get this merged for 7.33.0, so before Friday. |
|
The APM-related changes (pkg/trace) look fine to me, but I don't know where the skeletons are buried in that package. So, I've added the APM team to the QA list (by adding a label), and they can have a look and test it out in the QA process. |
Co-authored-by: Stephen Groat <[email protected]>
What does this PR do?
This PR fixes some of the code scanning alerts.
You can find the PR conversation we had with Agent Core team here: https://github.com/stephengroat/datadog-agent/pull/1/files
This PR fixes the following alerts
https://github.com/DataDog/datadog-agent/pull/9647/checks?check_run_id=4000934041
Feel free to let me know if this PR should be split into multiple small PRs
Motivation
What inspired you to submit this pull request?
Github Code Scanning
Additional Notes
Anything else we should know when reviewing?
Describe how to test your changes
Write here in detail how you have tested your changes
and instructions on how this should be tested in QA.
Describe or link instructions to set up environment
for testing, if the process requires more than just
running the agent on one of the supported platforms.
Checklist
changelog/no-changeloglabel has been applied.need-change/operatorandneed-change/helmlabels has been applied if applicable.team/..label has been applied, if known.Triagemilestone is set.Note: Adding GitHub labels is only possible for contributors with write access.