Skip to content

tpbase: codeql fixes#605

Merged
christos68k merged 3 commits intoopen-telemetry:mainfrom
grafana:korniltsev/tpbase_codeql_fixes
Jul 14, 2025
Merged

tpbase: codeql fixes#605
christos68k merged 3 commits intoopen-telemetry:mainfrom
grafana:korniltsev/tpbase_codeql_fixes

Conversation

@korniltsev
Copy link
Copy Markdown
Contributor

@korniltsev korniltsev commented Jul 14, 2025

Addressing the issues found in #602 as a separate change

Comment thread tpbase/libc_aarch64.go Fixed
Comment thread tpbase/libc_aarch64.go Fixed
@korniltsev korniltsev force-pushed the korniltsev/tpbase_codeql_fixes branch 2 times, most recently from aad09d7 to 0daaac4 Compare July 14, 2025 13:01
Comment thread armhelpers/arm_helpers.go Fixed
@korniltsev korniltsev force-pushed the korniltsev/tpbase_codeql_fixes branch 2 times, most recently from 321741e to 1d0c31a Compare July 14, 2025 13:14
@korniltsev korniltsev force-pushed the korniltsev/tpbase_codeql_fixes branch from 1d0c31a to f531129 Compare July 14, 2025 13:24
@korniltsev korniltsev marked this pull request as ready for review July 14, 2025 13:31
@korniltsev korniltsev requested review from a team as code owners July 14, 2025 13:31
Comment thread armhelpers/arm_helpers.go Outdated
return 0, false
}
return val, true
if val > math.MaxUint16 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about defining maxRegister = aa.V31 (that seems to be the largest valid register number) and use that here and in the other location?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, applied.
to be fair it looks like Reg(%d) case should be just treated as an error and return false. but I decided to keep it, can remove later.

Copy link
Copy Markdown
Contributor

@fabled fabled left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@christos68k christos68k merged commit c124776 into open-telemetry:main Jul 14, 2025
27 checks passed
gnurizen pushed a commit to parca-dev/opentelemetry-ebpf-profiler that referenced this pull request Sep 8, 2025
gnurizen pushed a commit to parca-dev/opentelemetry-ebpf-profiler that referenced this pull request Sep 8, 2025
gnurizen pushed a commit to parca-dev/opentelemetry-ebpf-profiler that referenced this pull request Sep 10, 2025
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.

4 participants