status check enforcement #6798
Closed
status check enforcement #6798
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tried making Status object enforce that it is checked in some way. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) and added it to the whitelist.
There was a problem hiding this comment.
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
pdillinger
reviewed
May 8, 2020
| code_ = std::move(s.code_); | ||
| s.code_ = kOk; | ||
| subcode_ = std::move(s.subcode_); | ||
| s.subcode_ = kNone; | ||
| retryable_ = s.retryable_; | ||
| data_loss_ = s.data_loss_; | ||
| scope_ = s.scope_; | ||
| scope_ = kIOErrorScopeFileSystem; | ||
| s.scope_ = kIOErrorScopeFileSystem; |
| @@ -204,14 +211,18 @@ inline IOStatus& IOStatus::operator=(IOStatus&& s) | |||
| #endif | |||
| { | |||
| if (this != &s) { | |||
| #ifdef ROCKSDB_ASSERT_STATUS_CHECKED | |||
| s.checked_ = true; | |||
There was a problem hiding this comment.
Why does Status ctor set to false but set to true after move?
There was a problem hiding this comment.
OK, that's obvious. Nevermind.
pdillinger
approved these changes
May 8, 2020
|
Thanks for the review! |
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary: Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) working and added it to the whitelist. Pull Request resolved: facebook/rocksdb#6798 Reviewed By: pdillinger Differential Revision: D21377404 Pulled By: ajkr fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e Signed-off-by: Changlong Chen <levisonchen@live.cn>
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary: Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) working and added it to the whitelist. Pull Request resolved: facebook/rocksdb#6798 Reviewed By: pdillinger Differential Revision: D21377404 Pulled By: ajkr fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e Signed-off-by: Changlong Chen <levisonchen@live.cn>
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary: Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) working and added it to the whitelist. Pull Request resolved: facebook/rocksdb#6798 Reviewed By: pdillinger Differential Revision: D21377404 Pulled By: ajkr fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e Signed-off-by: Changlong Chen <levisonchen@live.cn>
mm304321141
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 23, 2021
Summary: Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) working and added it to the whitelist. Pull Request resolved: facebook/rocksdb#6798 Reviewed By: pdillinger Differential Revision: D21377404 Pulled By: ajkr fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e Signed-off-by: Changlong Chen <levisonchen@live.cn>
mm304321141
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 23, 2021
Summary: Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) working and added it to the whitelist. Pull Request resolved: facebook/rocksdb#6798 Reviewed By: pdillinger Differential Revision: D21377404 Pulled By: ajkr fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e Signed-off-by: Changlong Chen <levisonchen@live.cn>
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 24, 2021
Summary: Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) working and added it to the whitelist. Pull Request resolved: facebook/rocksdb#6798 Reviewed By: pdillinger Differential Revision: D21377404 Pulled By: ajkr fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e Signed-off-by: Changlong Chen <levisonchen@live.cn>
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Sep 14, 2021
Summary: Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly. Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a whitelist. The effort appears significant to get each test to pass with this assertion, so I only fixed up enough to get one test (`options_test`) working and added it to the whitelist. Pull Request resolved: facebook/rocksdb#6798 Reviewed By: pdillinger Differential Revision: D21377404 Pulled By: ajkr fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e Signed-off-by: Changlong Chen <levisonchen@live.cn>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tried making Status object enforce that it is checked in some way. In cases it is not checked,
PermitUncheckedError()must be called explicitly.Added a way to run tests (
ASSERT_STATUS_CHECKED=1 make -j48 check) on awhitelist. The effort appears significant to get each test to pass with
this assertion, so I only fixed up enough to get one test (
options_test)working and added it to the whitelist.