KEMBAR78
Initialize CodeCache to NULL. by krk · Pull Request #1051 · async-profiler/async-profiler · GitHub
Skip to content

Conversation

krk
Copy link
Contributor

@krk krk commented Nov 8, 2024

Motivation and context

Prevents an unlikely uninitialized access.

How has this been tested?

make test

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Prevents an unlikely uninitialized access.
@apangin
Copy link
Member

apangin commented Nov 9, 2024

Prevents an unlikely uninitialized access

There is no uninitialized access in the given code. Even if there was, initialization to NULL would not be the right fix: at best, it would make the crash more deterministic, but it would still be a crash.

If this is an attempt to suppress a false positive finding of a static analyzer - this is fine: I don't mind trivial changes to satify static analyzer, if they have no impact on readability, performance or anything else, but let's be explicit in this case.

@krk
Copy link
Contributor Author

krk commented Nov 11, 2024

This initialization addresses the static analyzer finding: "variable 'cc' may be uninitialized when used here." While there’s no actual uninitialized access in the current code, setting cc to NULL serves as a defensive measure. This ensures a deterministic crash if cc were ever accessed before initialization, preventing silent corruption of CodeCache that could otherwise lead to incorrect results or crashes elsewhere or here.

Although a crash from uninitialized access isn’t currently possible here, initializing to NULL not only satisfies the static analyzer but also protects against potential future code changes that might unintentionally introduce uninitialized access to cc.

@apangin apangin merged commit f863502 into async-profiler:master Nov 11, 2024
6 checks passed
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.

2 participants