-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
fix clang analyzer warnings #4072
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to land this first, or do you want to fix the other error as well?
db/version_set.cc
Outdated
@@ -2758,7 +2758,7 @@ Status VersionSet::ProcessManifestWrites( | |||
break; | |||
} | |||
last_writer = *(it++); | |||
assert(last_writer != nullptr); | |||
assert(last_writer != nullptr && last_writer->cfd != nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe two asserts?
assert(last_writer != nullptr);
assert(last_writer->cfd != nullptr);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
the other error:
appear to have existed for a while now and does not break the clang analyzer contrun job. We can fix that separately. |
It's funny that this error does not break the cont build, while the other two warnings do. XD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miasantreble is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: clang analyze is giving the following warnings: > db/compaction_job.cc:1178:16: warning: Called C++ object pointer is null } else if (meta->smallest.size() > 0) { ^~~~~~~~~~~~~~~~~~~~~ db/compaction_job.cc:1201:33: warning: Access to field 'marked_for_compaction' results in a dereference of a null pointer (loaded from variable 'meta') meta->marked_for_compaction = sub_compact->builder->NeedCompact(); ~~~~ db/version_set.cc:2770:26: warning: Called C++ object pointer is null uint32_t cf_id = last_writer->cfd->GetID(); ^~~~~~~~~~~~~~~~~~~~~~~~~ Closes facebook#4072 Differential Revision: D8685852 Pulled By: miasantreble fbshipit-source-id: b0e2fd9dfc1cbba2317723e09886384b9b1c9085
clang analyze is giving the following warnings: