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

C++: add false positives to MissingCheckScanf test #12422

Merged
merged 2 commits into from
Mar 7, 2023

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Mar 7, 2023

See #12412 for the initial report.

@redsun82 redsun82 requested a review from geoffw0 March 7, 2023 10:57
@redsun82 redsun82 requested a review from a team as a code owner March 7, 2023 10:57
@github-actions github-actions bot added the C++ label Mar 7, 2023
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

LGTM.

void scan_and_static_variable() {
static int i;
scanf("%d", &i);
use(i); // GOOD [FALSE POSITIVE]: static variables are always 0-initialized
Copy link
Contributor

@geoffw0 geoffw0 Mar 7, 2023

Choose a reason for hiding this comment

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

This one's perhaps a bit of a maybe - while i should have been initialized it seems like there's a good chance in real world code we're still using an incorrect value in a case like this. Still, I gather this is motivated by a real world case where using the zero initialization / previously held value is OK, so that speaks for itself. We don't want false positives.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing is that a static variable will never have a random unexpected value: it will start out with 0, and then it will have whatever value gets written to it during the program execution, as written in the program itself. So yes, it's true this could still be a bug, but not the kind of bug that suddenly makes you try to load 4294967295 entries 😆

@redsun82 redsun82 merged commit bdad847 into main Mar 7, 2023
@redsun82 redsun82 deleted the redsun82/cpp-scanf-fp branch March 7, 2023 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants